Optimize CommandLineV1Flag type dispatch, validation, and FlagSaver. 1. Replace `IsOfType<T>()` (up to 6 virtual `TypeId()` calls + 6 indirect `op_(kFastTypeId)` calls) in `ChangedDirectly`, `CommandLineV1Flag::Typename()`, and `CommandLineV1Flag::InvokeValidator()` with direct `op == &absl_flags::FlagOps<T>` pointer comparisons. 2. Add `!inited_.load(std::memory_order_acquire)` fast paths to `CommandLineV1Flag::ValidateDefaultValue()`, `IsModified()`, `IsSpecifiedOnCommandLine()`, and `HasValidatorFn()`, avoiding lazily allocating and locking ~8,000 `absl::Mutex`es during flag initialization. 3. Return `nullptr` from `CommandLineV1Flag::SaveState()` when unmodified and restore via `CommandLineV1Flag::RestoreToInitialState()`, eliminating ~8,000 `Clone(op_, cur_)` + `V1FlagState` heap allocations/deletions per `FlagSaver` scope. PiperOrigin-RevId: 989386175
diff --git a/absl/flags/commandlineflag.cc b/absl/flags/commandlineflag.cc index 4140230..3b3b785 100644 --- a/absl/flags/commandlineflag.cc +++ b/absl/flags/commandlineflag.cc
@@ -26,6 +26,7 @@ absl::string_view CommandLineFlag::TypeName() const { return ""; } bool CommandLineFlag::IsRetired() const { return false; } +void CommandLineFlag::RestoreToInitialState() {} bool CommandLineFlag::ParseFrom(absl::string_view value, std::string* error) { return ParseFrom(value, flags_internal::SET_FLAGS_VALUE, flags_internal::kProgrammaticChange, *error);
diff --git a/absl/flags/commandlineflag.h b/absl/flags/commandlineflag.h index 9336d6d..aab7dac 100644 --- a/absl/flags/commandlineflag.h +++ b/absl/flags/commandlineflag.h
@@ -188,6 +188,9 @@ // or nullptr if flag does not support saving and restoring a state. virtual std::unique_ptr<flags_internal::FlagStateInterface> SaveState() = 0; + // Restores a flag that was in its initial unmodified state when saved. + virtual void RestoreToInitialState(); + // Copy-construct a new value of the flag's type in a memory referenced by // the dst based on the current flag's value. virtual void Read(void* absl_nonnull dst) const = 0;
diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc index 662b342..62d1be9 100644 --- a/absl/flags/internal/flag.cc +++ b/absl/flags/internal/flag.cc
@@ -59,11 +59,17 @@ namespace { // Currently we only validate flag values for user-defined flag types. -bool ShouldValidateFlagValue(FlagFastTypeId flag_type_id) { -#define DONT_VALIDATE(T, _) \ +bool ShouldValidateFlagValue(FlagOpFn op) { +#define DONT_VALIDATE_OP(T, _) \ + if (op == &FlagOps<T>) return false; + ABSL_FLAGS_INTERNAL_SUPPORTED_TYPES(DONT_VALIDATE_OP) +#undef DONT_VALIDATE_OP + + FlagFastTypeId flag_type_id = flags_internal::FastTypeId(op); +#define DONT_VALIDATE_ID(T, _) \ if (flag_type_id == absl::FastTypeId<T>()) return false; - ABSL_FLAGS_INTERNAL_SUPPORTED_TYPES(DONT_VALIDATE) -#undef DONT_VALIDATE + ABSL_FLAGS_INTERNAL_SUPPORTED_TYPES(DONT_VALIDATE_ID) +#undef DONT_VALIDATE_ID return true; } @@ -104,6 +110,39 @@ s_freelist->push_back(p); } +struct InitialDefaultEntry { + const FlagImpl* flag; + FlagDefaultSrc default_value; +}; + +ABSL_CONST_INIT std::vector<InitialDefaultEntry>* s_initial_defaults + ABSL_GUARDED_BY(FreelistMutex()) + ABSL_PT_GUARDED_BY(FreelistMutex()) = nullptr; + +void RecordInitialDefault(const FlagImpl* flag, FlagDefaultSrc default_value) { + absl::MutexLock l(FreelistMutex()); + if (!s_initial_defaults) { + s_initial_defaults = new std::vector<InitialDefaultEntry>; + } + for (const auto& entry : *s_initial_defaults) { + if (entry.flag == flag) return; + } + s_initial_defaults->push_back({flag, default_value}); +} + +FlagDefaultSrc GetInitialDefault(const FlagImpl* flag, + FlagDefaultSrc fallback_value) { + absl::MutexLock l(FreelistMutex()); + if (s_initial_defaults != nullptr) { + for (const auto& entry : *s_initial_defaults) { + if (entry.flag == flag) { + return entry.default_value; + } + } + } + return fallback_value; +} + } // namespace /////////////////////////////////////////////////////////////////////////////// @@ -374,6 +413,7 @@ } bool FlagImpl::IsSpecifiedOnCommandLine() const { + if (ABSL_PREDICT_TRUE(seq_lock_.IsUnmodified())) return false; absl::MutexLock l(DataGuard()); return on_command_line_; } @@ -464,14 +504,14 @@ 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 !ShouldValidateFlagValue(op_); } return false; } std::unique_ptr<FlagStateInterface> FlagImpl::SaveState() { if (ABSL_PREDICT_TRUE(CanSkipInitOnSave())) { - return std::make_unique<FlagState>(*this, default_value_); + return nullptr; } absl::MutexLock l(DataGuard()); @@ -506,6 +546,13 @@ return nullptr; } +void FlagImpl::RestoreToInitialState() { + if (ABSL_PREDICT_TRUE(seq_lock_.IsUnmodified())) { + return; + } + FlagState(*this, default_value_).Restore(); +} + bool FlagImpl::RestoreState(const FlagState& flag_state) { if (ABSL_PREDICT_TRUE(flag_state.counter_ < 0 && seq_lock_.IsUnmodified())) { return false; @@ -516,21 +563,26 @@ return false; } + const FlagDefaultSrc default_value = + (flag_state.counter_ < 0 && + DefaultKind() == FlagDefaultKind::kDynamicValue) + ? GetInitialDefault(this, default_value_) + : flag_state.value_.default_value; + switch (ValueStorageKind()) { case FlagValueStorageKind::kValueAndInitBit: case FlagValueStorageKind::kOneWordAtomic: - StoreValue( - flag_state.counter_ < 0 - ? static_cast<const void*>(&flag_state.value_.default_value) - : static_cast<const void*>(&flag_state.value_.one_word), - kProgrammaticChange); + StoreValue(flag_state.counter_ < 0 + ? static_cast<const void*>(&default_value) + : static_cast<const void*>(&flag_state.value_.one_word), + kProgrammaticChange); break; case FlagValueStorageKind::kSequenceLocked: case FlagValueStorageKind::kHeapAllocated: 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()); + (*default_value.gen_func)(init_value.get()); StoreValue(init_value.get(), kProgrammaticChange); } else { StoreValue(flag_state.value_.heap_allocated, kProgrammaticChange); @@ -652,7 +704,7 @@ void FlagImpl::Write(const void* src) { absl::MutexLock l(DataGuard()); - if (ShouldValidateFlagValue(flags_internal::FastTypeId(op_))) { + if (ShouldValidateFlagValue(op_)) { std::unique_ptr<void, DynValueDeleter> obj{flags_internal::Clone(op_, src), DynValueDeleter{op_}}; std::string ignored_error; @@ -717,6 +769,7 @@ default_value_.dynamic_value = tentative_value.release(); tentative_value.reset(old_value); } else { + RecordInitialDefault(this, default_value_); default_value_.dynamic_value = tentative_value.release(); def_kind_ = static_cast<uint8_t>(FlagDefaultKind::kDynamicValue); } @@ -734,11 +787,12 @@ } void FlagImpl::CheckDefaultValueParsingRoundtrip() const { - std::string v = DefaultValue(); + if (!ShouldValidateFlagValue(op_)) return; absl::MutexLock lock(DataGuard()); auto dst = MakeInitValue(); + std::string v = flags_internal::Unparse(op_, dst.get()); std::string error; if (!flags_internal::Parse(op_, v, dst.get(), &error)) { ABSL_INTERNAL_LOG(
diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h index 8baff43..f625456 100644 --- a/absl/flags/internal/flag.h +++ b/absl/flags/internal/flag.h
@@ -641,6 +641,8 @@ private: template <typename T> friend class Flag; + template <typename, bool> + friend class FlagRegistrar; friend class FlagState; // Ensures that `data_guard_` is initialized and returns it. @@ -718,6 +720,7 @@ // saving and restoring a state. std::unique_ptr<FlagStateInterface> SaveState() override ABSL_LOCKS_EXCLUDED(DataGuard()); + void RestoreToInitialState() override ABSL_LOCKS_EXCLUDED(DataGuard()); // Restores the flag state to the supplied state object. If there is // nothing to restore returns false. Otherwise returns true. @@ -939,8 +942,10 @@ public: constexpr explicit FlagRegistrar(Flag<T>& flag, const char* filename) : flag_(flag) { - if (do_register) - flags_internal::RegisterCommandLineFlag(flag_.impl_, filename); + if (do_register) { + flags_internal::RegisterCommandLineFlag( + flag_.impl_, flag_.impl_.filename_ == filename ? nullptr : filename); + } } FlagRegistrar OnUpdate(FlagCallbackFunc cb) && {
diff --git a/absl/flags/internal/private_handle_accessor.cc b/absl/flags/internal/private_handle_accessor.cc index 610d1ff..145b1b4 100644 --- a/absl/flags/internal/private_handle_accessor.cc +++ b/absl/flags/internal/private_handle_accessor.cc
@@ -36,6 +36,10 @@ return flag.SaveState(); } +void PrivateHandleAccessor::RestoreToInitialState(CommandLineFlag& flag) { + flag.RestoreToInitialState(); +} + bool PrivateHandleAccessor::IsSpecifiedOnCommandLine( const CommandLineFlag& flag) { return flag.IsSpecifiedOnCommandLine();
diff --git a/absl/flags/internal/private_handle_accessor.h b/absl/flags/internal/private_handle_accessor.h index bf4154c..fedf750 100644 --- a/absl/flags/internal/private_handle_accessor.h +++ b/absl/flags/internal/private_handle_accessor.h
@@ -39,6 +39,9 @@ // Access to CommandLineFlag::SaveState. static std::unique_ptr<FlagStateInterface> SaveState(CommandLineFlag& flag); + // Access to CommandLineFlag::RestoreToInitialState. + static void RestoreToInitialState(CommandLineFlag& flag); + // Access to CommandLineFlag::IsSpecifiedOnCommandLine. static bool IsSpecifiedOnCommandLine(const CommandLineFlag& flag);
diff --git a/absl/flags/parse.cc b/absl/flags/parse.cc index 940bf56..9b9502d 100644 --- a/absl/flags/parse.cc +++ b/absl/flags/parse.cc
@@ -335,13 +335,6 @@ #ifndef NDEBUG flags_internal::ForEachFlag([&](CommandLineFlag& flag) { if (flag.IsRetired()) return; - -#define ABSL_FLAGS_INTERNAL_IGNORE_TYPE(T, _) \ - if (flag.IsOfType<T>()) return; - - ABSL_FLAGS_INTERNAL_SUPPORTED_TYPES(ABSL_FLAGS_INTERNAL_IGNORE_TYPE) -#undef ABSL_FLAGS_INTERNAL_IGNORE_TYPE - flags_internal::PrivateHandleAccessor::CheckDefaultValueParsingRoundtrip( flag); });
diff --git a/absl/flags/reflection.cc b/absl/flags/reflection.cc index 13f210c..af8f037 100644 --- a/absl/flags/reflection.cc +++ b/absl/flags/reflection.cc
@@ -322,20 +322,26 @@ flags_internal::ForEachFlag([&](CommandLineFlag& flag) { if (auto flag_state = flags_internal::PrivateHandleAccessor::SaveState(flag)) { - backup_registry_.emplace_back(std::move(flag_state)); + backup_registry_.emplace(&flag, std::move(flag_state)); } }); } // Restores the saved flag states into the flag registry. void RestoreToRegistry() && { - for (const auto& flag_state : backup_registry_) { - std::move(*flag_state).Restore(); - } + flags_internal::ForEachFlag([&](CommandLineFlag& flag) { + if (auto it = backup_registry_.find(&flag); + it != backup_registry_.end()) { + std::move(*it->second).Restore(); + } else { + flags_internal::PrivateHandleAccessor::RestoreToInitialState(flag); + } + }); } private: - std::vector<std::unique_ptr<flags_internal::FlagStateInterface>> + absl::flat_hash_map<CommandLineFlag*, + std::unique_ptr<flags_internal::FlagStateInterface>> backup_registry_; };