Intern flag source filenames at registration into a 16-bit file ID. Every `absl::Flag` in writable data carries an 8-byte source filename pointer and a dynamic relocation entry, even though binaries define several times more flags than flag-defining source files. Pass the filename string literal from the static registrar into `absl::flags_internal::FlagImpl` at startup, recording each translation unit's filename in a process-wide pointer vector and storing its 1-based 16-bit ID in existing padding. Because all flags in a translation unit register consecutively from one static initializer and share the same `__FILE__` string literal, comparing pointers against only the last vector entry deduplicates per-file filenames in constant time without a hash table. Dropping the filename pointer shrinks `FlagImpl` from 64 bytes to 56 bytes, reduces scalar flags from 72 bytes to 64 bytes, and removes one dynamic relocation per flag. PiperOrigin-RevId: 1006216268
diff --git a/absl/flags/BUILD.bazel b/absl/flags/BUILD.bazel index 59771c7..a43151f 100644 --- a/absl/flags/BUILD.bazel +++ b/absl/flags/BUILD.bazel
@@ -240,6 +240,7 @@ "//absl/base:dynamic_annotations", "//absl/base:fast_type_id", "//absl/base:no_destructor", + "//absl/container:flat_hash_map", "//absl/memory", "//absl/meta:type_traits", "//absl/strings",
diff --git a/absl/flags/CMakeLists.txt b/absl/flags/CMakeLists.txt index 162b654..788ff0a 100644 --- a/absl/flags/CMakeLists.txt +++ b/absl/flags/CMakeLists.txt
@@ -201,6 +201,7 @@ absl::flags_commandlineflag_internal absl::flags_config absl::flags_marshalling + absl::flat_hash_map absl::meta absl::no_destructor absl::synchronization
diff --git a/absl/flags/commandlineflag_test.cc b/absl/flags/commandlineflag_test.cc index b6d01b2..35ce567 100644 --- a/absl/flags/commandlineflag_test.cc +++ b/absl/flags/commandlineflag_test.cc
@@ -16,7 +16,9 @@ #include "absl/flags/commandlineflag.h" #include <memory> +#include <optional> #include <string> +#include <vector> #include "gtest/gtest.h" #include "absl/flags/config.h" @@ -38,6 +40,21 @@ ABSL_FLAG(int, int_flag2, 201, ""); ABSL_FLAG(std::string, string_flag2, "dflt", ""); +using CustomIntAlias = int; +using CustomStringListAlias = std::vector<std::string>; + +struct CustomFlagUDT {}; +bool AbslParseFlag(absl::string_view, CustomFlagUDT*, std::string*) { + return true; +} +std::string AbslUnparseFlag(const CustomFlagUDT&) { return ""; } + +ABSL_FLAG(CustomIntAlias, int_alias_flag, 7, ""); +ABSL_FLAG(CustomStringListAlias, string_list_alias_flag, {}, ""); +ABSL_FLAG(std::vector<std::string>, string_vec_flag, {}, ""); +ABSL_FLAG(std::optional<int>, optional_int_flag, std::nullopt, ""); +ABSL_FLAG(CustomFlagUDT, custom_udt_flag, {}, ""); + namespace { namespace flags = absl::flags_internal; @@ -233,4 +250,67 @@ // EXPECT_EQ(err, "ERROR: int_flag is already set to 201"); } +void MakeEmptyString(void* dst) { new (dst) std::string(); } + +constexpr flags::FlagHelpArg kCompatHelpArg{flags::FlagHelpMsg("help"), + flags::FlagHelpKind::kLiteral}; +constexpr flags::FlagDefaultArg kCompatStringDefaultArg{ + flags::FlagDefaultSrc(&MakeEmptyString), flags::FlagDefaultKind::kGenFunc}; +constexpr absl::Flag<std::string> kCompat4ArgFlag{"compat4", "file", + kCompatHelpArg, + kCompatStringDefaultArg}; +constexpr absl::Flag<std::string> kCompat5ArgCommonFlag{ + "compat5_common", "std::string", "file", kCompatHelpArg, + kCompatStringDefaultArg}; +constexpr absl::Flag<std::string> kCompat5ArgCustomFlag{ + "compat5_custom", "UnlistedStringAlias", "file", kCompatHelpArg, + kCompatStringDefaultArg}; + +TEST_F(CommandLineFlagTest, TestTypeName) { + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + *absl::FindCommandLineFlag("int_flag")), + "int"); + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + *absl::FindCommandLineFlag("string_flag")), + "std::string"); + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + *absl::FindCommandLineFlag("string_vec_flag")), + "std::vector<std::string>"); + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + *absl::FindCommandLineFlag("optional_int_flag")), + "std::optional<int>"); + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + *absl::FindCommandLineFlag("int_alias_flag")), + "CustomIntAlias"); + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + *absl::FindCommandLineFlag("string_list_alias_flag")), + "CustomStringListAlias"); + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + *absl::FindCommandLineFlag("custom_udt_flag")), + "CustomFlagUDT"); + + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + absl::GetFlagReflectionHandle(kCompat4ArgFlag)), + ""); + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + absl::GetFlagReflectionHandle(kCompat5ArgCommonFlag)), + "std::string"); + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + absl::GetFlagReflectionHandle(kCompat5ArgCustomFlag)), + ""); +} + +TEST(CommandLineFlagStrippedTest, TestStrippedTypeName) { +#if ABSL_FLAGS_STRIP_NAMES + EXPECT_EQ(absl::GetFlagReflectionHandle(FLAGS_int_flag).Name(), ""); + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + absl::GetFlagReflectionHandle(FLAGS_int_flag)), + ""); + EXPECT_EQ(absl::GetFlagReflectionHandle(FLAGS_custom_udt_flag).Name(), ""); + EXPECT_EQ(flags::PrivateHandleAccessor::TypeName( + absl::GetFlagReflectionHandle(FLAGS_custom_udt_flag)), + ""); +#endif +} + } // namespace
diff --git a/absl/flags/flag.h b/absl/flags/flag.h index 2e9a9e6..ccbd08a 100644 --- a/absl/flags/flag.h +++ b/absl/flags/flag.h
@@ -207,6 +207,9 @@ #define ABSL_FLAG_IMPL_FLAGNAME(txt) "" #define ABSL_FLAG_IMPL_TYPENAME(txt) "" #define ABSL_FLAG_IMPL_FILENAME() "" +#define ABSL_FLAG_IMPL_NAME_ARG(name) \ + absl::flags_internal::FlagNameArg{"", absl::flags_internal::kEmptyFlagTypeId} +#define ABSL_FLAG_IMPL_DECLARE_NAME_METHODS(name, Type) #define ABSL_FLAG_IMPL_REGISTRAR(T, flag) \ absl::flags_internal::FlagRegistrar<T, false>(ABSL_FLAG_IMPL_FLAG_PTR(flag), \ nullptr) @@ -214,6 +217,18 @@ #define ABSL_FLAG_IMPL_FLAGNAME(txt) txt #define ABSL_FLAG_IMPL_TYPENAME(txt) txt #define ABSL_FLAG_IMPL_FILENAME() __FILE__ +#define ABSL_FLAG_IMPL_NAME_ARG(name) \ + absl::flags_internal::MakeFlagNameArg<AbslFlagDefaultGenFor##name>() +#define ABSL_FLAG_IMPL_DECLARE_NAME_METHODS(name, Type) \ + static constexpr const char* AbslFlagName() { \ + return #name; \ + } \ + static constexpr absl::string_view AbslFlagNameView() { \ + return absl::string_view(#name, sizeof(#name) - 1); \ + } \ + static constexpr absl::string_view AbslFlagTypeName() { \ + return absl::string_view(#Type, sizeof(#Type) - 1); \ + } #define ABSL_FLAG_IMPL_REGISTRAR(T, flag) \ absl::flags_internal::FlagRegistrar<T, true>(ABSL_FLAG_IMPL_FLAG_PTR(flag), \ __FILE__) @@ -241,7 +256,7 @@ /* default value argument. That keeps temporaries alive */ \ /* long enough for NonConst to work correctly. */ \ static constexpr absl::string_view Value( \ - absl::string_view absl_flag_help ABSL_ATTRIBUTE_LIFETIME_BOUND = \ + absl::string_view absl_flag_help ABSL_ATTRIBUTE_LIFETIME_BOUND = \ ABSL_FLAG_IMPL_FLAGHELP(txt)) { \ return absl_flag_help; \ } \ @@ -258,6 +273,7 @@ static void Gen(void* absl_flag_default_loc) { \ new (absl_flag_default_loc) Type(AbslFlagDefaultGenFor##name{}.value); \ } \ + ABSL_FLAG_IMPL_DECLARE_NAME_METHODS(name, Type) \ }; // ABSL_FLAG_IMPL @@ -265,17 +281,16 @@ // Note: Name of registrar object is not arbitrary. It is used to "grab" // global name for FLAGS_no<flag_name> symbol, thus preventing the possibility // of defining two flags with names foo and nofoo. -#define ABSL_FLAG_IMPL(Type, name, default_value, help) \ - extern ::absl::Flag<Type> FLAGS_##name; \ - namespace absl /* block flags in namespaces */ {} \ - ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value) \ - ABSL_FLAG_IMPL_DECLARE_HELP_WRAPPER(name, help) \ - ABSL_CONST_INIT absl::Flag<Type> FLAGS_##name{ \ - ABSL_FLAG_IMPL_FLAGNAME(#name), ABSL_FLAG_IMPL_TYPENAME(#Type), \ - ABSL_FLAG_IMPL_FILENAME(), ABSL_FLAG_IMPL_HELP_ARG(name), \ - ABSL_FLAG_IMPL_DEFAULT_ARG(Type, name)}; \ - extern absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name; \ - absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name = \ +#define ABSL_FLAG_IMPL(Type, name, default_value, help) \ + extern ::absl::Flag<Type> FLAGS_##name; \ + namespace absl /* block flags in namespaces */ {} \ + ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value) \ + ABSL_FLAG_IMPL_DECLARE_HELP_WRAPPER(name, help) \ + ABSL_CONST_INIT absl::Flag<Type> FLAGS_##name{ \ + ABSL_FLAG_IMPL_NAME_ARG(name), ABSL_FLAG_IMPL_HELP_ARG(name), \ + ABSL_FLAG_IMPL_DEFAULT_ARG(Type, name)}; \ + extern absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name; \ + absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name = \ ABSL_FLAG_IMPL_REGISTRAR(Type, FLAGS_##name) // ABSL_RETIRED_FLAG
diff --git a/absl/flags/flag_test.cc b/absl/flags/flag_test.cc index 5cacf64..7022eca 100644 --- a/absl/flags/flag_test.cc +++ b/absl/flags/flag_test.cc
@@ -137,6 +137,17 @@ flags::FlagValueStorageKind::kSequenceLocked); EXPECT_EQ(flags::StorageKind<absl::uint128>(), flags::FlagValueStorageKind::kSequenceLocked); + + // These sizes are not hard limits, but we want to keep per-flag size as + // small as possible because production binaries may define thousands of + // flags. + if (sizeof(void*) == 8) { + EXPECT_EQ(sizeof(flags::FlagImpl), 56); + EXPECT_EQ(sizeof(absl::Flag<bool>), 64); + EXPECT_EQ(sizeof(absl::Flag<int>), 64); + EXPECT_EQ(sizeof(absl::Flag<int64_t>), 64); + EXPECT_EQ(sizeof(absl::Flag<double>), 64); + } } // -------------------------------------------------------------------- @@ -148,17 +159,16 @@ using int128 = absl::int128; using uint128 = absl::uint128; -#define DEFINE_CONSTRUCTED_FLAG(T, dflt, dflt_kind) \ - constexpr flags::FlagDefaultArg f1default##T{ \ - flags::FlagDefaultSrc{dflt}, flags::FlagDefaultKind::dflt_kind}; \ - constexpr absl::Flag<T> f1##T{"f1", #T, "file", help_arg, f1default##T}; \ - ABSL_CONST_INIT absl::Flag<T> f2##T { \ - "f2", #T, "file", \ - {flags::FlagHelpMsg(&TestHelpMsg), flags::FlagHelpKind::kGenFunc}, \ - flags::FlagDefaultArg { \ - flags::FlagDefaultSrc(&TestMakeDflt<T>), \ - flags::FlagDefaultKind::kGenFunc \ - } \ +#define DEFINE_CONSTRUCTED_FLAG(T, dflt, dflt_kind) \ + constexpr flags::FlagDefaultArg f1default##T{ \ + flags::FlagDefaultSrc{dflt}, flags::FlagDefaultKind::dflt_kind}; \ + constexpr absl::Flag<T> f1##T{"f1", help_arg, f1default##T}; \ + ABSL_CONST_INIT absl::Flag<T> f2##T { \ + "f2", {flags::FlagHelpMsg(&TestHelpMsg), flags::FlagHelpKind::kGenFunc}, \ + flags::FlagDefaultArg { \ + flags::FlagDefaultSrc(&TestMakeDflt<T>), \ + flags::FlagDefaultKind::kGenFunc \ + } \ } DEFINE_CONSTRUCTED_FLAG(bool, true, kOneWord); @@ -175,13 +185,18 @@ DEFINE_CONSTRUCTED_FLAG(int128, 13, kGenFunc); DEFINE_CONSTRUCTED_FLAG(uint128, 14, kGenFunc); +constexpr absl::Flag<String> f_compat_5arg{ + "f_compat", "std::string", "file", help_arg, + flags::FlagDefaultArg{flags::FlagDefaultSrc(&TestMakeDflt<String>), + flags::FlagDefaultKind::kGenFunc}}; + template <typename T> bool TestConstructionFor(const absl::Flag<T>& f1, absl::Flag<T>& f2) { EXPECT_EQ(absl::GetFlagReflectionHandle(f1).Name(), "f1"); EXPECT_EQ(absl::GetFlagReflectionHandle(f1).Help(), "literal help"); - EXPECT_EQ(absl::GetFlagReflectionHandle(f1).Filename(), "file"); + EXPECT_EQ(absl::GetFlagReflectionHandle(f1).Filename(), ""); - flags::FlagRegistrar<T, false>(ABSL_FLAG_IMPL_FLAG_PTR(f2), nullptr) + flags::FlagRegistrar<T, false>(ABSL_FLAG_IMPL_FLAG_PTR(f2), "file") .OnUpdate(TestCallback); EXPECT_EQ(absl::GetFlagReflectionHandle(f2).Name(), "f2"); @@ -207,6 +222,10 @@ TEST_CONSTRUCTED_FLAG(UDT); TEST_CONSTRUCTED_FLAG(int128); TEST_CONSTRUCTED_FLAG(uint128); + EXPECT_EQ(absl::GetFlagReflectionHandle(f_compat_5arg).Name(), "f_compat"); + EXPECT_EQ(absl::GetFlagReflectionHandle(f_compat_5arg).Help(), + "literal help"); + EXPECT_EQ(absl::GetFlagReflectionHandle(f_compat_5arg).Filename(), ""); } // -------------------------------------------------------------------- @@ -1397,3 +1416,46 @@ #endif } // namespace + +ABSL_FLAG(int, concurrent_saver_test_flag, 0, + "test flag for concurrent FlagSaver"); + +namespace { + +TEST_F(FlagTest, ConcurrentFlagSaverAndSetFlag) { + std::atomic<bool> stop{false}; + std::thread writer([&stop]() { + int i = 1; + while (!stop.load(std::memory_order_relaxed)) { + absl::SetFlag(&FLAGS_concurrent_saver_test_flag, i++); + } + }); + for (int iter = 0; iter < 50; ++iter) { + absl::FlagSaver saver; + (void)absl::GetFlag(FLAGS_concurrent_saver_test_flag); + } + stop.store(true, std::memory_order_relaxed); + writer.join(); + EXPECT_GE(absl::GetFlag(FLAGS_concurrent_saver_test_flag), 0); +} + +TEST_F(FlagTest, FilenameInterning) { + constexpr const char* kFileA = "dir/intern_test_a.cc"; + constexpr const char* kFileB = "dir/intern_test_b.cc"; + + absl::Flag<int32_t> f_a1{"fa1", help_arg, f1defaultint32_t}; + absl::Flag<int32_t> f_a2{"fa2", help_arg, f1defaultint32_t}; + absl::Flag<int32_t> f_b{"fb", help_arg, f1defaultint32_t}; + + flags::FlagRegistrar<int32_t, false>(ABSL_FLAG_IMPL_FLAG_PTR(f_a1), kFileA); + flags::FlagRegistrar<int32_t, false>(ABSL_FLAG_IMPL_FLAG_PTR(f_a2), kFileA); + flags::FlagRegistrar<int32_t, false>(ABSL_FLAG_IMPL_FLAG_PTR(f_b), kFileB); + // Duplicate registration preserves the first filename for ODR diagnostics. + flags::FlagRegistrar<int32_t, false>(ABSL_FLAG_IMPL_FLAG_PTR(f_a1), kFileB); + + EXPECT_EQ(f_a1.Filename(), "dir/intern_test_a.cc"); + EXPECT_EQ(f_a2.Filename(), "dir/intern_test_a.cc"); + EXPECT_EQ(f_b.Filename(), "dir/intern_test_b.cc"); +} + +} // namespace
diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc index 662b342..6c3d18c 100644 --- a/absl/flags/internal/flag.cc +++ b/absl/flags/internal/flag.cc
@@ -23,6 +23,7 @@ #include <array> #include <atomic> #include <cstring> +#include <limits> #include <memory> #include <string> #include <typeinfo> @@ -39,6 +40,7 @@ #include "absl/base/no_destructor.h" #include "absl/base/optimization.h" #include "absl/base/thread_annotations.h" +#include "absl/container/flat_hash_map.h" #include "absl/flags/config.h" #include "absl/flags/internal/commandlineflag.h" #include "absl/flags/usage_config.h" @@ -104,6 +106,91 @@ s_freelist->push_back(p); } +template <typename K, typename V> +class ThreadSafeMap { + public: + V Get(const K& key) { + absl::MutexLock l(mutex_); + auto it = map_.find(key); + return it != map_.end() ? it->second : V{}; + } + + V Set(const K& key, V new_val) { + absl::MutexLock l(mutex_); + return std::exchange(map_[key], new_val); + } + + template <typename Factory> + V GetOrCreate(const K& key, Factory factory) { + absl::MutexLock l(mutex_); + V& val = map_[key]; + if (val == V{}) { + val = factory(); + } + return val; + } + + void Increment(const K& key) { + absl::MutexLock l(mutex_); + ++map_[key]; + } + + private: + absl::Mutex mutex_; + absl::flat_hash_map<K, V> map_ ABSL_GUARDED_BY(mutex_); +}; + +FlagCallback* GetOrCreateCallback(const FlagImpl* flag) { + static absl::NoDestructor<ThreadSafeMap<const FlagImpl*, FlagCallback*>> + callbacks; + return callbacks->GetOrCreate(flag, [] { return new FlagCallback; }); +} + +ThreadSafeMap<const FlagImpl*, void*>& DynamicDefaults() { + static absl::NoDestructor<ThreadSafeMap<const FlagImpl*, void*>> + dynamic_defaults; + return *dynamic_defaults; +} + +void* GetDynamicDefault(const FlagImpl* flag) { + return DynamicDefaults().Get(flag); +} + +void* SetDynamicDefault(const FlagImpl* flag, void* new_val) { + return DynamicDefaults().Set(flag, new_val); +} + +ThreadSafeMap<const FlagImpl*, int64_t>& OverflowModCounts() { + static absl::NoDestructor<ThreadSafeMap<const FlagImpl*, int64_t>> + overflow_mod_counts; + return *overflow_mod_counts; +} + +int64_t GetOverflowModCount(const FlagImpl* flag) { + return OverflowModCounts().Get(flag); +} + +void IncrementOverflowModCount(const FlagImpl* flag) { + OverflowModCounts().Increment(flag); +} + +struct FilenameTable { + absl::Mutex mutex; + std::vector<const char*> files ABSL_GUARDED_BY(mutex); +}; + +FilenameTable& GlobalFilenames() { + static absl::NoDestructor<FilenameTable> filenames; + return *filenames; +} + +const char* GetFilenameById(uint16_t id) { + if (id == 0) return ""; + auto& table = GlobalFilenames(); + absl::MutexLock l(table.mutex); + return table.files[id - 1]; +} + } // namespace /////////////////////////////////////////////////////////////////////////////// @@ -210,10 +297,34 @@ return (reinterpret_cast<mask_t>(ptr_) & mask) != 0; } +void FlagImpl::SetFilename(const char* filename) { + // Keep the first registration's filename so FlagRegistry::RegisterFlag can + // report both files when dynamic symbol interposition merges duplicate flag + // definitions across shared libraries. + if (file_id_ != 0 || filename == nullptr) return; + auto& table = GlobalFilenames(); + absl::MutexLock l(table.mutex); + // All flags in a translation unit register consecutively from one + // .init_array initializer and share one merged .rodata.str1.1 __FILE__ + // string literal, so pointer equality against the last entry deduplicates + // per-TU filenames in O(1) without strcmp or a hash table. + if (!table.files.empty() && table.files.back() == filename) { + file_id_ = static_cast<uint16_t>(table.files.size()); + return; + } + if (ABSL_PREDICT_FALSE(table.files.size() >= + std::numeric_limits<uint16_t>::max())) { + ABSL_INTERNAL_LOG(FATAL, "Too many flag filenames"); + } + table.files.push_back(filename); + file_id_ = static_cast<uint16_t>(table.files.size()); +} + void FlagImpl::Init() { new (&data_guard_) absl::Mutex; - auto def_kind = static_cast<FlagDefaultKind>(def_kind_); + FlagDefaultKind def_kind = + mu_state_.load(std::memory_order_relaxed).DefaultKind(); switch (ValueStorageKind()) { case FlagValueStorageKind::kValueAndInitBit: @@ -242,6 +353,7 @@ // during initialization. assert(def_kind == FlagDefaultKind::kGenFunc); (*default_value_.gen_func)(AtomicBufferValue()); + SeqLock().MarkInitialized(); break; } case FlagValueStorageKind::kHeapAllocated: @@ -256,7 +368,6 @@ std::memory_order_release); break; } - seq_lock_.MarkInitialized(); } absl::Mutex& FlagImpl::DataGuard() const { @@ -294,9 +405,9 @@ std::unique_ptr<void, DynValueDeleter> FlagImpl::MakeInitValue() const { void* res = nullptr; - switch (DefaultKind()) { + switch (mu_state_.load(std::memory_order_relaxed).DefaultKind()) { case FlagDefaultKind::kDynamicValue: - res = flags_internal::Clone(op_, default_value_.dynamic_value); + res = flags_internal::Clone(op_, GetDynamicDefault(this)); break; case FlagDefaultKind::kGenFunc: res = flags_internal::Alloc(op_); @@ -309,6 +420,17 @@ return {res, DynValueDeleter{op_}}; } +void FlagImpl::RecordMutation() { + MuState s = mu_state_.load(std::memory_order_relaxed); + if (s.mod_count < kMaxInlineModCount) { + ++s.mod_count; + } else { + IncrementOverflowModCount(this); + } + s.modified = true; + mu_state_.store(s, std::memory_order_release); +} + void FlagImpl::StoreValue(const void* src, ValueSource source) { switch (ValueStorageKind()) { case FlagValueStorageKind::kValueAndInitBit: @@ -317,11 +439,10 @@ int64_t one_word_val = OneWordValue().load(std::memory_order_acquire); std::memcpy(&one_word_val, src, Sizeof(op_)); OneWordValue().store(one_word_val, std::memory_order_release); - seq_lock_.IncrementModificationCount(); break; } case FlagValueStorageKind::kSequenceLocked: { - seq_lock_.Write(AtomicBufferValue(), src, Sizeof(op_)); + SeqLock().Write(AtomicBufferValue(), src, Sizeof(op_)); break; } case FlagValueStorageKind::kHeapAllocated: @@ -345,19 +466,24 @@ } PtrStorage().store(ptr_value, std::memory_order_release); - seq_lock_.IncrementModificationCount(); break; } - modified_ = true; + RecordMutation(); InvokeCallback(); } absl::string_view FlagImpl::Name() const { return name_; } -absl::string_view FlagImpl::TypeName() const { return type_name_; } +absl::string_view FlagImpl::TypeName() const { + if (type_id_ != kCustomFlagTypeId) { + return kCommonFlagTypeNames[type_id_]; + } + return absl::string_view(name_ + std::strlen(name_) + 1); +} std::string FlagImpl::Filename() const { - return flags_internal::GetUsageConfig().normalize_filename(filename_); + return flags_internal::GetUsageConfig().normalize_filename( + GetFilenameById(file_id_)); } std::string FlagImpl::Help() const { @@ -370,12 +496,16 @@ } int64_t FlagImpl::ModificationCount() const { - return seq_lock_.ModificationCount(); + const uint8_t mod_count = + mu_state_.load(std::memory_order_relaxed).mod_count; + if (ABSL_PREDICT_TRUE(mod_count < kMaxInlineModCount)) { + return mod_count; + } + return kMaxInlineModCount + GetOverflowModCount(this); } bool FlagImpl::IsSpecifiedOnCommandLine() const { - absl::MutexLock l(DataGuard()); - return on_command_line_; + return mu_state_.load(std::memory_order_relaxed).on_command_line; } std::string FlagImpl::DefaultValue() const { @@ -414,20 +544,22 @@ void FlagImpl::SetCallback(const FlagCallbackFunc mutation_callback) { absl::MutexLock l(DataGuard()); - if (callback_ == nullptr) { - callback_ = new FlagCallback; - } - callback_->func = mutation_callback; + FlagCallback* cb = GetOrCreateCallback(this); + cb->func = mutation_callback; + MuState s = mu_state_.load(std::memory_order_relaxed); + s.has_callback = true; + mu_state_.store(s, std::memory_order_release); InvokeCallback(); } void FlagImpl::InvokeCallback() const { - if (!callback_) return; + if (!mu_state_.load(std::memory_order_relaxed).has_callback) return; + FlagCallback* cb_entry = GetOrCreateCallback(this); // Make a copy of the C-style function pointer that we are about to invoke // before we release the lock guarding it. - FlagCallbackFunc cb = callback_->func; + FlagCallbackFunc cb = cb_entry->func; // If the flag has a mutation callback this function invokes it. While the // callback is being invoked the primary flag's mutex is unlocked and it is @@ -441,30 +573,31 @@ // completed. Requires that *primary_lock be held in exclusive mode; it may be // released and reacquired by the implementation. MutexRelock relock(DataGuard()); - absl::MutexLock lock(callback_->guard); + absl::MutexLock lock(cb_entry->guard); 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. +// Returns true if an unmodified absl::Flag 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; + const MuState state = mu_state_.load(std::memory_order_acquire); + if (state.mod_count != 0) return false; + const FlagDefaultKind def_kind = state.DefaultKind(); 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; + return def_kind == FlagDefaultKind::kOneWord; case FlagValueStorageKind::kSequenceLocked: // Always uses a default-generator function when unmodified. - return true; + return def_kind == FlagDefaultKind::kGenFunc; 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 def_kind == FlagDefaultKind::kGenFunc && + !ShouldValidateFlagValue(flags_internal::FastTypeId(op_)); } return false; } @@ -476,8 +609,9 @@ absl::MutexLock l(DataGuard()); - bool modified = modified_; - bool on_command_line = on_command_line_; + const MuState state = mu_state_.load(std::memory_order_relaxed); + bool modified = state.modified; + bool on_command_line = state.on_command_line; switch (ValueStorageKind()) { case FlagValueStorageKind::kValueAndInitBit: case FlagValueStorageKind::kOneWordAtomic: { @@ -489,7 +623,7 @@ void* cloned = flags_internal::Alloc(op_); // Read is guaranteed to be successful because we hold the lock. bool success = - seq_lock_.TryRead(cloned, AtomicBufferValue(), Sizeof(op_)); + SeqLock().TryRead(cloned, AtomicBufferValue(), Sizeof(op_)); assert(success); static_cast<void>(success); return std::make_unique<FlagState>(*this, cloned, modified, @@ -507,7 +641,7 @@ } bool FlagImpl::RestoreState(const FlagState& flag_state) { - if (ABSL_PREDICT_TRUE(flag_state.counter_ < 0 && seq_lock_.IsUnmodified())) { + if (ABSL_PREDICT_TRUE(flag_state.counter_ < 0 && IsUnmodifiedFast())) { return false; } @@ -538,8 +672,10 @@ break; } - modified_ = flag_state.modified_; - on_command_line_ = flag_state.on_command_line_; + MuState s = mu_state_.load(std::memory_order_relaxed); + s.modified = flag_state.modified_; + s.on_command_line = flag_state.on_command_line_; + mu_state_.store(s, std::memory_order_release); return true; } @@ -558,6 +694,14 @@ return OffsetValue<std::atomic<uint64_t>>(); } +flags_internal::SequenceLock& FlagImpl::SeqLock() const { + assert(ValueStorageKind() == FlagValueStorageKind::kSequenceLocked); + size_t num_words = + flags_internal::AlignUp(Sizeof(op_), sizeof(uint64_t)) / sizeof(uint64_t); + return *reinterpret_cast<flags_internal::SequenceLock*>(AtomicBufferValue() + + num_words); +} + std::atomic<int64_t>& FlagImpl::OneWordValue() const { assert(ValueStorageKind() == FlagValueStorageKind::kOneWordAtomic || ValueStorageKind() == FlagValueStorageKind::kValueAndInitBit); @@ -638,13 +782,13 @@ void FlagImpl::ReadSequenceLockedData(void* dst) const { size_t size = Sizeof(op_); // Attempt to read using the sequence lock. - if (ABSL_PREDICT_TRUE(seq_lock_.TryRead(dst, AtomicBufferValue(), size))) { + if (ABSL_PREDICT_TRUE(SeqLock().TryRead(dst, AtomicBufferValue(), size))) { return; } // We failed due to contention. Acquire the lock to prevent contention // and try again. absl::ReaderMutexLock l(DataGuard()); - bool success = seq_lock_.TryRead(dst, AtomicBufferValue(), size); + bool success = SeqLock().TryRead(dst, AtomicBufferValue(), size); assert(success); static_cast<void>(success); } @@ -687,13 +831,15 @@ StoreValue(tentative_value.get(), source); if (source == kCommandLine) { - on_command_line_ = true; + MuState s = mu_state_.load(std::memory_order_relaxed); + s.on_command_line = true; + mu_state_.store(s, std::memory_order_release); } break; } case SET_FLAG_IF_DEFAULT: { // set the flag's value, but only if it hasn't been set by someone else - if (modified_) { + if (mu_state_.load(std::memory_order_relaxed).modified) { // TODO(rogeeff): review and fix this semantic. Currently we do not fail // in this case if flag is modified. This is misleading since the flag's // value is not updated even though we return true. @@ -712,19 +858,23 @@ auto tentative_value = TryParse(value, err); if (!tentative_value) return false; - if (DefaultKind() == FlagDefaultKind::kDynamicValue) { - void* old_value = default_value_.dynamic_value; - default_value_.dynamic_value = tentative_value.release(); + void* new_default = tentative_value.release(); + void* old_value = SetDynamicDefault(this, new_default); + MuState s = mu_state_.load(std::memory_order_relaxed); + const bool was_modified = s.modified; + if (s.DefaultKind() == FlagDefaultKind::kDynamicValue) { tentative_value.reset(old_value); } else { - default_value_.dynamic_value = tentative_value.release(); - def_kind_ = static_cast<uint8_t>(FlagDefaultKind::kDynamicValue); + s.def_kind = static_cast<uint8_t>(FlagDefaultKind::kDynamicValue); + mu_state_.store(s, std::memory_order_release); } - if (!modified_) { + if (!was_modified) { // Need to set both default value *and* current, in this case. - StoreValue(default_value_.dynamic_value, source); - modified_ = false; + StoreValue(new_default, source); + s = mu_state_.load(std::memory_order_relaxed); + s.modified = false; + mu_state_.store(s, std::memory_order_release); } break; }
diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h index 8baff43..34fcab3 100644 --- a/absl/flags/internal/flag.h +++ b/absl/flags/internal/flag.h
@@ -154,7 +154,6 @@ return static_cast<ptrdiff_t>(reinterpret_cast<intptr_t>( op(FlagOp::kValueOffset, nullptr, nullptr, nullptr))); } - // Returns an address of RTTI's typeid(T). template <typename T> inline const std::type_info* GenRuntimeTypeId() { @@ -166,7 +165,7 @@ } /////////////////////////////////////////////////////////////////////////////// -// Flag help auxiliary structs. +// Flag help and name/type-name auxiliary structs. // This is help argument for absl::Flag encapsulating the string literal pointer // or pointer to function generating it as well as enum discriminating two @@ -184,6 +183,113 @@ } }; +inline constexpr uint8_t kEmptyFlagTypeId = 0; +inline constexpr uint8_t kCustomFlagTypeId = 31; + +// Common macro-stringified `#Type` spellings stored as a 5-bit ID in `FlagImpl` +// so common flags avoid emitting a per-flag type-name string. Entry 0 (`""`) is +// used when flag names are stripped or omitted; ID 31 (`kCustomFlagTypeId`) +// indicates that `name_` points to a packed `"<name>\0<type_name>\0"` array. +inline constexpr absl::string_view kCommonFlagTypeNames[] = { + "", + "bool", + "int", + "int16_t", + "uint16_t", + "int32_t", + "uint32_t", + "int64_t", + "uint64_t", + "size_t", + "float", + "double", + "std::string", + "string", + "std::vector<std::string>", + "absl::Duration", + "absl::Time", + "absl::int128", + "absl::uint128", + "std::int32_t", + "std::int64_t", + "std::optional<bool>", + "std::optional<int>", + "std::optional<int32_t>", + "std::optional<uint32_t>", + "std::optional<int64_t>", + "std::optional<uint64_t>", + "std::optional<size_t>", + "std::optional<double>", + "std::optional<std::string>", + "std::optional<absl::Duration>", +}; +static_assert(sizeof(kCommonFlagTypeNames) / sizeof(kCommonFlagTypeNames[0]) == + kCustomFlagTypeId, + "kCommonFlagTypeNames must have kCustomFlagTypeId entries"); + +constexpr bool ConstexprStrEq(absl::string_view a, absl::string_view b) { + if (a.size() != b.size()) return false; + for (size_t i = 0; i < a.size(); ++i) { + if (a[i] != b[i]) return false; + } + return true; +} + +constexpr uint8_t LookupCommonFlagTypeId(absl::string_view type_name) { + for (size_t i = 0; i < kCustomFlagTypeId; ++i) { + if (ConstexprStrEq(kCommonFlagTypeNames[i], type_name)) { + return static_cast<uint8_t>(i); + } + } + return kCustomFlagTypeId; +} + +constexpr uint8_t LookupCommonOrEmptyFlagTypeId(const char* type_name) { + if (type_name == nullptr) return kEmptyFlagTypeId; + size_t len = 0; + while (type_name[len] != '\0') ++len; + const uint8_t id = LookupCommonFlagTypeId(absl::string_view(type_name, len)); + return id == kCustomFlagTypeId ? kEmptyFlagTypeId : id; +} + +struct FlagNameArg { + const char* name; + uint8_t type_id; +}; + +template <size_t N1, size_t N2, size_t... I1, size_t... I2> +constexpr FixedCharArray<N1 + N2 + 2> PackNameAndTypeImpl( + absl::string_view name, absl::string_view type_name, + std::index_sequence<I1...>, std::index_sequence<I2...>) { + return (void)name, (void)type_name, + FixedCharArray<N1 + N2 + 2>( + {{name[I1]..., '\0', type_name[I2]..., '\0'}}); +} + +template <typename Gen> +constexpr auto PackNameAndType() { + constexpr absl::string_view kName = Gen::AbslFlagNameView(); + constexpr absl::string_view kType = Gen::AbslFlagTypeName(); + return PackNameAndTypeImpl<kName.size(), kType.size()>( + kName, kType, std::make_index_sequence<kName.size()>{}, + std::make_index_sequence<kType.size()>{}); +} + +template <typename Gen> +struct PackedFlagNameAndType { + static constexpr auto value = PackNameAndType<Gen>(); +}; + +template <typename Gen> +constexpr FlagNameArg MakeFlagNameArg() { + constexpr uint8_t kTypeId = LookupCommonFlagTypeId(Gen::AbslFlagTypeName()); + if constexpr (kTypeId != kCustomFlagTypeId) { + return {Gen::AbslFlagName(), kTypeId}; + } else { + return {PackedFlagNameAndType<Gen>::value.value, kCustomFlagTypeId}; + } +} + template <typename Gen, size_t N = Gen::Value().size()> constexpr FixedCharArray<N + 1> HelpStringAsArray(int) { return FixedCharArray<N + 1>::FromLiteralString( @@ -437,7 +543,7 @@ template <typename T> struct FlagValue<T, FlagValueStorageKind::kValueAndInitBit> : FlagOneWordValue { constexpr FlagValue() : FlagOneWordValue(0) {} - bool Get(const SequenceLock&, T& dst) const { + bool Get(T& dst) const { int64_t storage = value.load(std::memory_order_acquire); if (ABSL_PREDICT_FALSE(storage == 0)) { // This assert is to ensure that the initialization inside FlagImpl::Init @@ -458,7 +564,7 @@ template <typename T> struct FlagValue<T, FlagValueStorageKind::kOneWordAtomic> : FlagOneWordValue { constexpr FlagValue() : FlagOneWordValue() {} - bool Get(const SequenceLock&, T& dst) const { + bool Get(T& dst) const { int64_t one_word_val = value.load(std::memory_order_acquire); if (ABSL_PREDICT_FALSE(one_word_val == FlagOneWordValue::Uninitialized())) { return false; @@ -476,8 +582,17 @@ // value once it is initialized. template <typename T> struct FlagValue<T, FlagValueStorageKind::kSequenceLocked> { - bool Get(const SequenceLock& lock, T& dst) const { - return lock.TryRead(&dst, value_words, sizeof(T)); + constexpr FlagValue() : FlagValue(std::make_index_sequence<kNumWords>{}) {} + + bool Get(T& dst) const { + // FlagImpl::SeqLock() computes `&seq_lock` via `AtomicBufferValue() + + // kNumWords`, so verify no padding or fields sit between `value_words` + // and `seq_lock`. + static_assert( + offsetof(FlagValue, seq_lock) - offsetof(FlagValue, value_words) == + kNumWords * sizeof(std::atomic<uint64_t>), + "Unexpected memory layout of sequence-locked FlagValue"); + return seq_lock.TryRead(&dst, value_words, sizeof(T)); } static constexpr int kNumWords = @@ -485,6 +600,12 @@ alignas(T) alignas( std::atomic<uint64_t>) std::atomic<uint64_t> value_words[kNumWords]; + flags_internal::SequenceLock seq_lock; + + private: + template <size_t... Is> + constexpr explicit FlagValue(std::index_sequence<Is...>) + : value_words{(static_cast<void>(Is), uint64_t{0})...} {} }; // This specialization represents the storage of flag values types with the @@ -531,7 +652,7 @@ // ensure Init is done before any access to the flag value. constexpr FlagValue() : FlagMaskedPointerValue(&buffer[0]) {} - bool Get(const SequenceLock&, T& dst) const { + bool Get(T& dst) const { MaskedPointer ptr_value = value.load(std::memory_order_acquire); if (ABSL_PREDICT_TRUE(ptr_value.AllowsUnprotectedRead())) { @@ -579,24 +700,40 @@ #pragma GCC diagnostic ignored "-Wnon-virtual-dtor" #endif class FlagImpl final : public CommandLineFlag { + private: + static constexpr uint8_t kMaxInlineModCount = 7; + + struct MuState { + uint8_t def_kind : 2; + bool modified : 1; + bool on_command_line : 1; + bool has_callback : 1; + uint8_t mod_count : 3; + + constexpr FlagDefaultKind DefaultKind() const { + return static_cast<FlagDefaultKind>(def_kind); + } + }; + static_assert(sizeof(MuState) == 1, "MuState must be 1 byte"); + static_assert(std::atomic<MuState>::is_always_lock_free, + "std::atomic<MuState> must be lock-free"); + public: - constexpr FlagImpl(const char* name, const char* type_name, - const char* filename, FlagOpFn op, FlagHelpArg help, + constexpr FlagImpl(FlagNameArg name, FlagOpFn op, FlagHelpArg help, FlagValueStorageKind value_kind, FlagDefaultArg default_arg) - : name_(name), - type_name_(type_name), - filename_(filename), + : name_(name.name), op_(op), help_(help.source), + default_value_(default_arg.source), + data_guard_{}, + init_control_{}, + file_id_(0), help_source_kind_(static_cast<uint8_t>(help.kind)), value_storage_kind_(static_cast<uint8_t>(value_kind)), - def_kind_(static_cast<uint8_t>(default_arg.kind)), - modified_(false), - on_command_line_(false), - callback_(nullptr), - default_value_(default_arg.source), - data_guard_{} {} + type_id_(name.type_id), + mu_state_(MuState{static_cast<uint8_t>(default_arg.kind), false, false, + false, 0}) {} // Constant access methods int64_t ReadOneWord() const ABSL_LOCKS_EXCLUDED(DataGuard()); @@ -623,6 +760,7 @@ // Mutating access methods void Write(const void* src) ABSL_LOCKS_EXCLUDED(DataGuard()); + void SetFilename(const char* filename); // Interfaces to operate on callbacks. void SetCallback(const FlagCallbackFunc mutation_callback) @@ -664,6 +802,9 @@ // The same as above, but used for sequencelock-protected storage. std::atomic<uint64_t>* AtomicBufferValue() const; + // Accessor for the SequenceLock inside FlagValue<T, kSequenceLocked>. + flags_internal::SequenceLock& SeqLock() const; + // This is an accessor for a value stored as one word atomic. Returns a // mutable reference to an atomic value. std::atomic<int64_t>& OneWordValue() const; @@ -690,10 +831,6 @@ FlagValueStorageKind ValueStorageKind() const { return static_cast<FlagValueStorageKind>(value_storage_kind_); } - FlagDefaultKind DefaultKind() const - ABSL_EXCLUSIVE_LOCKS_REQUIRED(DataGuard()) { - return static_cast<FlagDefaultKind>(def_kind_); - } // CommandLineFlag interface implementation absl::string_view Name() const override; @@ -701,8 +838,7 @@ std::string Filename() const override; std::string Help() const override; FlagFastTypeId TypeId() const override; - bool IsSpecifiedOnCommandLine() const override - ABSL_LOCKS_EXCLUDED(DataGuard()); + bool IsSpecifiedOnCommandLine() const override; std::string DefaultValue() const override ABSL_LOCKS_EXCLUDED(DataGuard()); std::string CurrentValue() const override ABSL_LOCKS_EXCLUDED(DataGuard()); bool ValidateInputValue(absl::string_view value) const override @@ -711,7 +847,17 @@ ABSL_LOCKS_EXCLUDED(DataGuard()); int64_t ModificationCount() const ABSL_EXCLUSIVE_LOCKS_REQUIRED(DataGuard()); + // Lock-free check used by SaveState() and RestoreState() to skip lazy + // initialization on flags that have never been mutated. Safe without + // DataGuard() because all mutations update mu_state_ under DataGuard() + // with release semantics after writing the flag value, and observing a + // zero modification count means the flag still holds its initial + // compile-time state. + bool IsUnmodifiedFast() const { + return mu_state_.load(std::memory_order_acquire).mod_count == 0; + } bool CanSkipInitOnSave() const; + void RecordMutation() ABSL_EXCLUSIVE_LOCKS_REQUIRED(DataGuard()); // Interfaces to save and restore flags to/from persistent state. // Returns current flag state or nullptr if flag does not support @@ -730,58 +876,39 @@ // Immutable flag's state. - // Flags name passed to ABSL_FLAG as second arg. + // Flags name passed to ABSL_FLAG as second arg (followed by NUL-terminated + // type name when `type_id_ == kCustomFlagTypeId`). const char* const name_; - - // Flags type passed to ABSL_FLAG as first arg. - const char* const type_name_; - - // The file name where ABSL_FLAG resides. - const char* const filename_; // Type-specific operations vtable. const FlagOpFn op_; // Help message literal or function to generate it. const FlagHelpMsg help_; - // Indicates if help message was supplied as literal or generator func. - const uint8_t help_source_kind_ : 1; - // Kind of storage this flag is using for the flag's value. - const uint8_t value_storage_kind_ : 2; - - uint8_t : 0; // The bytes containing the const bitfields must not be - // shared with bytes containing the mutable bitfields. - - // Mutable flag's state (guarded by `data_guard_`). - - // def_kind_ is not guard by DataGuard() since it is accessed in Init without - // locks. - uint8_t def_kind_ : 2; - // Has this flag's value been modified? - bool modified_ : 1 ABSL_GUARDED_BY(DataGuard()); - // Has this flag been specified on command line. - bool on_command_line_ : 1 ABSL_GUARDED_BY(DataGuard()); + // Initial default value source specified in ABSL_FLAG. Dynamic default + // overrides set via SET_FLAGS_DEFAULT are stored in a side table. + const FlagDefaultSrc default_value_; + // Reserved space for an absl::Mutex to guard flag data, initialized in Init. + alignas(absl::Mutex) mutable unsigned char data_guard_[sizeof(absl::Mutex)]; // Unique tag for absl::call_once call to initialize this flag. absl::once_flag init_control_; - // Sequence lock / mutation counter. - flags_internal::SequenceLock seq_lock_; + // Deduplicated filename index assigned on registration (0 = unset). + uint16_t file_id_; - // Optional flag's callback and absl::Mutex to guard the invocations. - FlagCallback* callback_ ABSL_GUARDED_BY(DataGuard()); - // Either a pointer to the function generating the default value based on the - // value specified in ABSL_FLAG or pointer to the dynamically set default - // value via SetCommandLineOptionWithMode. def_kind_ is used to distinguish - // these two cases. - FlagDefaultSrc default_value_; + // Indicates if help message was supplied as literal or generator func. + const uint8_t help_source_kind_ : 1; + // Kind of storage this flag is using for the flag's value. + const uint8_t value_storage_kind_ : 2; + // Index into `kCommonFlagTypeNames`, or `kCustomFlagTypeId` when `name_` + // stores `"<name>\0<type_name>\0"`. + const uint8_t type_id_ : 5; - // This is reserved space for an absl::Mutex to guard flag data. It will be - // initialized in FlagImpl::Init via placement new. - // We can't use "absl::Mutex data_guard_", since this class is not literal. - // We do not want to use "absl::Mutex* data_guard_", since this would require - // heap allocation during initialization, which is both slows program startup - // and can fail. Using reserved space + placement new allows us to avoid both - // problems. - alignas(absl::Mutex) mutable unsigned char data_guard_[sizeof(absl::Mutex)]; + // Atomic state bitfields (def_kind, modified, on_command_line, has_callback, + // and saturating 3-bit mod_count where 7 indicates overflow in side table). + // All writes occur under DataGuard() with release semantics; SaveState() and + // RestoreState() perform lock-free acquire loads to fast-path unmodified + // flags without triggering lazy initialization. + std::atomic<MuState> mu_state_; }; #if defined(__GNUC__) && !defined(__clang__) #pragma GCC diagnostic pop @@ -794,11 +921,23 @@ template <typename T> class Flag { public: - constexpr Flag(const char* name, const char* type_name, const char* filename, - FlagHelpArg help, const FlagDefaultArg default_arg) - : impl_(name, type_name, filename, &FlagOps<T>, help, - flags_internal::StorageKind<T>(), default_arg), + constexpr Flag(FlagNameArg name, FlagHelpArg help, + const FlagDefaultArg default_arg) + : impl_(name, &FlagOps<T>, help, flags_internal::StorageKind<T>(), + default_arg), value_() {} + constexpr Flag(const char* name, FlagHelpArg help, + const FlagDefaultArg default_arg) + : Flag(FlagNameArg{name, kEmptyFlagTypeId}, help, default_arg) {} + constexpr Flag(const char* name, const char* /*filename*/, FlagHelpArg help, + const FlagDefaultArg default_arg) + : Flag(FlagNameArg{name, kEmptyFlagTypeId}, help, default_arg) {} + // Kept for backwards compatibility. + constexpr Flag(const char* name, const char* type_name, + const char* /*filename*/, FlagHelpArg help, + const FlagDefaultArg default_arg) + : Flag(FlagNameArg{name, LookupCommonOrEmptyFlagTypeId(type_name)}, help, + default_arg) {} // CommandLineFlag interface absl::string_view Name() const { return impl_.Name(); } @@ -829,7 +968,7 @@ impl_.AssertValidType(absl::FastTypeId<T>(), &GenRuntimeTypeId<T>); #endif - if (ABSL_PREDICT_FALSE(!value_.Get(impl_.seq_lock_, u.value))) { + if (ABSL_PREDICT_FALSE(!value_.Get(u.value))) { impl_.Read(&u.value); } return std::move(u.value); @@ -939,8 +1078,12 @@ public: constexpr explicit FlagRegistrar(Flag<T>& flag, const char* filename) : flag_(flag) { - if (do_register) + if (filename != nullptr) { + flag_.impl_.SetFilename(filename); + } + if (do_register) { flags_internal::RegisterCommandLineFlag(flag_.impl_, filename); + } } FlagRegistrar OnUpdate(FlagCallbackFunc cb) && {
diff --git a/absl/flags/internal/sequence_lock.h b/absl/flags/internal/sequence_lock.h index 1729d7f..f3c95ec 100644 --- a/absl/flags/internal/sequence_lock.h +++ b/absl/flags/internal/sequence_lock.h
@@ -66,11 +66,6 @@ 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. @@ -124,25 +119,6 @@ lock_.store(orig_seq + 2, std::memory_order_release); } - // Return the number of times that Write() has been called. - // - // REQUIRES: This must be externally synchronized against concurrent calls to - // `Write()` or `IncrementModificationCount()`. - // REQUIRES: `MarkInitialized()` must have been previously called. - int64_t ModificationCount() const { - int64_t val = lock_.load(std::memory_order_relaxed); - assert(val != kUninitialized && (val & 1) == 0); - return val / 2; - } - - // REQUIRES: This must be externally synchronized against concurrent calls to - // `Write()` or `ModificationCount()`. - // REQUIRES: `MarkInitialized()` must have been previously called. - void IncrementModificationCount() { - int64_t val = lock_.load(std::memory_order_relaxed); - assert(val != kUninitialized); - lock_.store(val + 2, std::memory_order_relaxed); - } private: // Perform the equivalent of "memcpy(dst, src, size)", but using relaxed