Skip initializing, locking, and cloning unmodified flags in FlagSaver. GoogleTest constructs an absl::FlagSaver around every test, which previously forced lazy initialization, mutex acquisition, and value cloning across every registered flag in the binary even when almost none are modified. Instead, record unmodified flags without initializing them, acquiring their mutex, or cloning their values, and reconstruct their initial default value on restoration only if the flag was mutated while saved. PiperOrigin-RevId: 1006034347
diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc index 0a1dff2..662b342 100644 --- a/absl/flags/internal/flag.cc +++ b/absl/flags/internal/flag.cc
@@ -129,7 +129,15 @@ on_command_line_(on_command_line), counter_(counter) {} + FlagState(FlagImpl& flag_impl, FlagDefaultSrc default_value) + : flag_impl_(flag_impl), + value_(default_value), + modified_(false), + on_command_line_(false), + counter_(-1) {} + ~FlagState() override { + if (counter_ < 0) return; if (flag_impl_.ValueStorageKind() != FlagValueStorageKind::kHeapAllocated && flag_impl_.ValueStorageKind() != FlagValueStorageKind::kSequenceLocked) return; @@ -153,9 +161,11 @@ union SavedValue { explicit SavedValue(void* v) : heap_allocated(v) {} explicit SavedValue(int64_t v) : one_word(v) {} + explicit SavedValue(FlagDefaultSrc v) : default_value(v) {} void* heap_allocated; int64_t one_word; + FlagDefaultSrc default_value; } value_; bool modified_; bool on_command_line_; @@ -435,7 +445,35 @@ cb(); } +// Returns true if an unmodified absl::Flag (i.e. `seq_lock_.IsUnmodified()`) +// can skip lazy initialization and state allocation in `SaveState()`, and +// instead reconstructs from the provided default value if modified before +// restoration. +bool FlagImpl::CanSkipInitOnSave() const { + if (!seq_lock_.IsUnmodified()) return false; + switch (ValueStorageKind()) { + case FlagValueStorageKind::kValueAndInitBit: + case FlagValueStorageKind::kOneWordAtomic: + // Only `kOneWord` defaults are inlinable compile-time constants; other + // kinds may have dynamic defaults or side effects. + return static_cast<FlagDefaultKind>(def_kind_) == + FlagDefaultKind::kOneWord; + case FlagValueStorageKind::kSequenceLocked: + // Always uses a default-generator function when unmodified. + return true; + case FlagValueStorageKind::kHeapAllocated: + // Built-in types have side-effect-free defaults; user-defined types may + // have custom construction or validation side effects. + return !ShouldValidateFlagValue(flags_internal::FastTypeId(op_)); + } + return false; +} + std::unique_ptr<FlagStateInterface> FlagImpl::SaveState() { + if (ABSL_PREDICT_TRUE(CanSkipInitOnSave())) { + return std::make_unique<FlagState>(*this, default_value_); + } + absl::MutexLock l(DataGuard()); bool modified = modified_; @@ -469,6 +507,10 @@ } bool FlagImpl::RestoreState(const FlagState& flag_state) { + if (ABSL_PREDICT_TRUE(flag_state.counter_ < 0 && seq_lock_.IsUnmodified())) { + return false; + } + absl::MutexLock l(DataGuard()); if (flag_state.counter_ == ModificationCount()) { return false; @@ -477,11 +519,22 @@ switch (ValueStorageKind()) { case FlagValueStorageKind::kValueAndInitBit: case FlagValueStorageKind::kOneWordAtomic: - StoreValue(&flag_state.value_.one_word, kProgrammaticChange); + StoreValue( + flag_state.counter_ < 0 + ? static_cast<const void*>(&flag_state.value_.default_value) + : static_cast<const void*>(&flag_state.value_.one_word), + kProgrammaticChange); break; case FlagValueStorageKind::kSequenceLocked: case FlagValueStorageKind::kHeapAllocated: - StoreValue(flag_state.value_.heap_allocated, kProgrammaticChange); + if (flag_state.counter_ < 0) { + std::unique_ptr<void, DynValueDeleter> init_value( + flags_internal::Alloc(op_), DynValueDeleter{op_}); + (*flag_state.value_.default_value.gen_func)(init_value.get()); + StoreValue(init_value.get(), kProgrammaticChange); + } else { + StoreValue(flag_state.value_.heap_allocated, kProgrammaticChange); + } break; }
diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h index 8ef95ab..8baff43 100644 --- a/absl/flags/internal/flag.h +++ b/absl/flags/internal/flag.h
@@ -284,7 +284,8 @@ } template <typename ValueT, typename GenT, - std::enable_if_t<std::is_integral_v<ValueT>, int> = ((void)GenT{}, 0)> + std::enable_if_t<std::is_arithmetic_v<ValueT>, int> = ((void)GenT{}, + 0)> constexpr FlagDefaultArg DefaultArg(int) { return {FlagDefaultSrc(GenT{}.value), FlagDefaultKind::kOneWord}; } @@ -710,6 +711,7 @@ ABSL_LOCKS_EXCLUDED(DataGuard()); int64_t ModificationCount() const ABSL_EXCLUSIVE_LOCKS_REQUIRED(DataGuard()); + bool CanSkipInitOnSave() const; // Interfaces to save and restore flags to/from persistent state. // Returns current flag state or nullptr if flag does not support
diff --git a/absl/flags/internal/sequence_lock.h b/absl/flags/internal/sequence_lock.h index da04ff4..1729d7f 100644 --- a/absl/flags/internal/sequence_lock.h +++ b/absl/flags/internal/sequence_lock.h
@@ -66,6 +66,12 @@ lock_.store(0, std::memory_order_release); } + // Returns true if the flag is either uninitialized or initialized with zero + // modifications. + bool IsUnmodified() const { + return lock_.load(std::memory_order_acquire) <= 0; + } + // Copy "size" bytes of data from "src" to "dst", protected as a read-side // critical section of the sequence lock. //
diff --git a/absl/flags/reflection.cc b/absl/flags/reflection.cc index e7eeaa3..13f210c 100644 --- a/absl/flags/reflection.cc +++ b/absl/flags/reflection.cc
@@ -191,6 +191,7 @@ if (registry.finalized_flags_.load(std::memory_order_acquire)) { for (const auto& i : registry.flat_flags_) visitor(*i); + return; } FlagRegistryLock frl(registry);
diff --git a/absl/flags/reflection_test.cc b/absl/flags/reflection_test.cc index 99a3525..0fb902d 100644 --- a/absl/flags/reflection_test.cc +++ b/absl/flags/reflection_test.cc
@@ -139,6 +139,12 @@ static int counter = 0; ABSL_FLAG(int, test_flag_13, 200, "").OnUpdate([]() { counter++; }); ABSL_FLAG(CustomUDT, test_flag_14, {}, ""); +static int runtime_bool_default_eval_count = 0; +static bool GetRuntimeBoolDefault() { + ++runtime_bool_default_eval_count; + return true; +} +ABSL_FLAG(bool, test_flag_15, GetRuntimeBoolDefault(), ""); namespace { @@ -270,4 +276,16 @@ EXPECT_EQ(absl::GetFlag(FLAGS_test_flag_08), 9876543); } +TEST_F(ReflectionTest, TestFlagSaverEagerlyInitsRuntimeDefaultIntegralFlag) { + { + absl::FlagSaver s; + const int evals_after_save = runtime_bool_default_eval_count; + EXPECT_GE(evals_after_save, 1); + absl::SetFlag(&FLAGS_test_flag_15, false); + EXPECT_EQ(runtime_bool_default_eval_count, evals_after_save); + EXPECT_FALSE(absl::GetFlag(FLAGS_test_flag_15)); + } + EXPECT_TRUE(absl::GetFlag(FLAGS_test_flag_15)); +} + } // namespace