about summary refs log tree commit diff
path: root/absl/flags
diff options
context:
space:
mode:
authorAbseil Team <absl-team@google.com>2020-03-03T19·22-0800
committerAndy Soffer <asoffer@google.com>2020-03-03T22·32-0500
commitb19ba96766db08b1f32605cb4424a0e7ea0c7584 (patch)
treec4ba295b067b000b9d84410ec81e0095715641a5 /absl/flags
parent06f0e767d13d4d68071c4fc51e25724e0fc8bc74 (diff)
Export of internal Abseil changes
--
a3e58c1870a9626039f4d178d2d599319bd9f8a8 by Matt Kulukundis <kfm@google.com>:

Allow MakeCordFromExternal to take a zero arg releaser.

PiperOrigin-RevId: 298650274

--
01897c4a9bb99f3dc329a794019498ad345ddebd by Samuel Benzaquen <sbenza@google.com>:

Reduce library bloat for absl::Flag by moving the definition of base virtual functions to a .cc file.
This removes the duplicate symbols in user translation units and  has the side effect of moving the vtable definition too (re key function)

PiperOrigin-RevId: 298617920

--
190f0d3782c63aed01046886d7fbc1be5bca2de9 by Derek Mauro <dmauro@google.com>:

Import GitHub #596: Unbreak stacktrace code for UWP apps

PiperOrigin-RevId: 298600834

--
cd5cf6f8c87b35b85a9584e94da2a99057345b73 by Gennadiy Rozental <rogeeff@google.com>:

Use union of heap allocated pointer, one word atomic and two word atomic to represent flags value.

Any type T, which is trivially copy-able and with with sizeof(T) <= 8, will be stored in atomic int64_t.
Any type T, which is trivially copy-able and with with 8 < sizeof(T) <= 16, will be stored in atomic AlignedTwoWords.

We also introducing value storage type to distinguish these cases.

PiperOrigin-RevId: 298497200

--
f8fe7bd53bfed601f002f521e34ab4bc083fc28b by Matthew Brown <matthewbr@google.com>:

Ensure a deep copy and proper equality on absl::Status::ErasePayload

PiperOrigin-RevId: 298482742

--
a5c9ccddf4b04f444e3f7e27dbc14faf1fcb5373 by Gennadiy Rozental <rogeeff@google.com>:

Change ChunkIterator implementation to use fixed capacity collection of CordRep*. We can now assume that depth never exceeds 91. That makes comparison operator exception safe.

I've tested that with this CL we do not observe an overhead of chunk_end. Compiler optimized this iterator completely.

PiperOrigin-RevId: 298458472

--
327ea5e8910bc388b03389c730763f9823abfce5 by Abseil Team <absl-team@google.com>:

Minor cleanups in b-tree code:
- Rename some variables: fix issues of different param names between definition/declaration, move away from `x` as a default meaningless variable name.
- Make init_leaf/init_internal be non-static methods (they already take the node as the first parameter).
- In internal_emplace/try_shrink, update root/rightmost the same way as in insert_unique/insert_multi.
- Replace a TODO with a comment.

PiperOrigin-RevId: 298432836

--
8020ce9ec8558ee712d9733ae3d660ac1d3ffe1a by Abseil Team <absl-team@google.com>:

Guard against unnecessary copy in case the buffer is empty. This is important in cases were the user is explicitly tuning their chunks to match PiecewiseChunkSize().

PiperOrigin-RevId: 298366044

--
89324441d1c0c697c90ba7d8fc63639805fcaa9d by Abseil Team <absl-team@google.com>:

Internal change

PiperOrigin-RevId: 298219363
GitOrigin-RevId: a3e58c1870a9626039f4d178d2d599319bd9f8a8
Change-Id: I28dffc684b6fd0292b94807b88ec6664d5d0e183
Diffstat (limited to 'absl/flags')
-rw-r--r--absl/flags/BUILD.bazel4
-rw-r--r--absl/flags/CMakeLists.txt3
-rw-r--r--absl/flags/flag.h1
-rw-r--r--absl/flags/flag_benchmark.cc8
-rw-r--r--absl/flags/flag_test.cc143
-rw-r--r--absl/flags/internal/commandlineflag.cc30
-rw-r--r--absl/flags/internal/commandlineflag.h6
-rw-r--r--absl/flags/internal/flag.cc107
-rw-r--r--absl/flags/internal/flag.h208
9 files changed, 325 insertions, 185 deletions
diff --git a/absl/flags/BUILD.bazel b/absl/flags/BUILD.bazel
index cdb4e7e8fe..9166f74c1b 100644
--- a/absl/flags/BUILD.bazel
+++ b/absl/flags/BUILD.bazel
@@ -45,6 +45,7 @@ cc_library(
         "//absl/base:config",
         "//absl/base:core_headers",
         "//absl/memory",
+        "//absl/meta:type_traits",
         "//absl/strings",
         "//absl/synchronization",
     ],
@@ -130,6 +131,9 @@ cc_library(
 
 cc_library(
     name = "handle",
+    srcs = [
+        "internal/commandlineflag.cc",
+    ],
     hdrs = [
         "internal/commandlineflag.h",
     ],
diff --git a/absl/flags/CMakeLists.txt b/absl/flags/CMakeLists.txt
index 1d25f0ded8..01cf09b1b5 100644
--- a/absl/flags/CMakeLists.txt
+++ b/absl/flags/CMakeLists.txt
@@ -33,6 +33,7 @@ absl_cc_library(
     absl::flags_handle
     absl::flags_registry
     absl::synchronization
+    absl::meta
   PUBLIC
 )
 
@@ -117,6 +118,8 @@ absl_cc_library(
 absl_cc_library(
   NAME
     flags_handle
+  SRCS
+    "internal/commandlineflag.cc"
   HDRS
     "internal/commandlineflag.h"
   COPTS
diff --git a/absl/flags/flag.h b/absl/flags/flag.h
index cff02c1fcb..bb917654d5 100644
--- a/absl/flags/flag.h
+++ b/absl/flags/flag.h
@@ -148,7 +148,6 @@ class Flag {
     return GetImpl()->template IsOfType<U>();
   }
   T Get() const { return GetImpl()->Get(); }
-  bool AtomicGet(T* v) const { return GetImpl()->AtomicGet(v); }
   void Set(const T& v) { GetImpl()->Set(v); }
   void SetCallback(const flags_internal::FlagCallbackFunc mutation_callback) {
     GetImpl()->SetCallback(mutation_callback);
diff --git a/absl/flags/flag_benchmark.cc b/absl/flags/flag_benchmark.cc
index 87f731704c..ff95bb5d7b 100644
--- a/absl/flags/flag_benchmark.cc
+++ b/absl/flags/flag_benchmark.cc
@@ -109,3 +109,11 @@ namespace {
 BENCHMARKED_TYPES(BM_GetFlag)
 
 }  // namespace
+
+#define InvokeGetFlag(T)                                               \
+  T AbslInvokeGetFlag##T() { return absl::GetFlag(FLAGS_##T##_flag); } \
+  int odr##T = (benchmark::DoNotOptimize(AbslInvokeGetFlag##T), 1);
+
+BENCHMARKED_TYPES(InvokeGetFlag)
+
+// To veiw disassembly use: gdb ${BINARY}  -batch -ex "disassemble /s $FUNC"
diff --git a/absl/flags/flag_test.cc b/absl/flags/flag_test.cc
index 4984d28403..1e01b49cc4 100644
--- a/absl/flags/flag_test.cc
+++ b/absl/flags/flag_test.cc
@@ -49,28 +49,6 @@ void* TestMakeDflt() {
 }
 void TestCallback() {}
 
-template <typename T>
-bool TestConstructionFor() {
-  constexpr flags::FlagHelpArg help_arg{flags::FlagHelpMsg("literal help"),
-                                        flags::FlagHelpKind::kLiteral};
-  constexpr flags::Flag<T> f1("f1", "file", help_arg, &TestMakeDflt<T>);
-  EXPECT_EQ(f1.Name(), "f1");
-  EXPECT_EQ(f1.Help(), "literal help");
-  EXPECT_EQ(f1.Filename(), "file");
-
-  ABSL_CONST_INIT static flags::Flag<T> f2(
-      "f2", "file",
-      {flags::FlagHelpMsg(&TestHelpMsg), flags::FlagHelpKind::kGenFunc},
-      &TestMakeDflt<T>);
-  flags::FlagRegistrar<T, false>(&f2).OnUpdate(TestCallback);
-
-  EXPECT_EQ(f2.Name(), "f2");
-  EXPECT_EQ(f2.Help(), "dynamic help");
-  EXPECT_EQ(f2.Filename(), "file");
-
-  return true;
-}
-
 struct UDT {
   UDT() = default;
   UDT(const UDT&) = default;
@@ -98,19 +76,103 @@ class FlagTest : public testing::Test {
   }
 };
 
+struct S1 {
+  S1() = default;
+  S1(const S1&) = default;
+  int32_t f1;
+  int64_t f2;
+};
+
+struct S2 {
+  S2() = default;
+  S2(const S2&) = default;
+  int64_t f1;
+  double f2;
+};
+
+TEST_F(FlagTest, Traits) {
+  EXPECT_EQ(flags::FlagValue::Kind<int>(),
+            flags::FlagValueStorageKind::kOneWordAtomic);
+  EXPECT_EQ(flags::FlagValue::Kind<bool>(),
+            flags::FlagValueStorageKind::kOneWordAtomic);
+  EXPECT_EQ(flags::FlagValue::Kind<double>(),
+            flags::FlagValueStorageKind::kOneWordAtomic);
+  EXPECT_EQ(flags::FlagValue::Kind<int64_t>(),
+            flags::FlagValueStorageKind::kOneWordAtomic);
+
+#if defined(ABSL_FLAGS_INTERNAL_ATOMIC_DOUBLE_WORD)
+  EXPECT_EQ(flags::FlagValue::Kind<S1>(),
+            flags::FlagValueStorageKind::kTwoWordsAtomic);
+  EXPECT_EQ(flags::FlagValue::Kind<S2>(),
+            flags::FlagValueStorageKind::kTwoWordsAtomic);
+#else
+  EXPECT_EQ(flags::FlagValue::Kind<S1>(),
+            flags::FlagValueStorageKind::kHeapAllocated);
+  EXPECT_EQ(flags::FlagValue::Kind<S2>(),
+            flags::FlagValueStorageKind::kHeapAllocated);
+#endif
+
+  EXPECT_EQ(flags::FlagValue::Kind<std::string>(),
+            flags::FlagValueStorageKind::kHeapAllocated);
+  EXPECT_EQ(flags::FlagValue::Kind<std::vector<std::string>>(),
+            flags::FlagValueStorageKind::kHeapAllocated);
+}
+
+// --------------------------------------------------------------------
+
+constexpr flags::FlagHelpArg help_arg{flags::FlagHelpMsg("literal help"),
+                                      flags::FlagHelpKind::kLiteral};
+
+using String = std::string;
+
+#define DEFINE_CONSTRUCTED_FLAG(T)                                          \
+  constexpr flags::Flag<T> f1##T("f1", "file", help_arg, &TestMakeDflt<T>); \
+  ABSL_CONST_INIT flags::Flag<T> f2##T(                                     \
+      "f2", "file",                                                         \
+      {flags::FlagHelpMsg(&TestHelpMsg), flags::FlagHelpKind::kGenFunc},    \
+      &TestMakeDflt<T>)
+
+#define TEST_CONSTRUCTED_FLAG(T) TestConstructionFor(f1##T, &f2##T);
+
+DEFINE_CONSTRUCTED_FLAG(bool);
+DEFINE_CONSTRUCTED_FLAG(int16_t);
+DEFINE_CONSTRUCTED_FLAG(uint16_t);
+DEFINE_CONSTRUCTED_FLAG(int32_t);
+DEFINE_CONSTRUCTED_FLAG(uint32_t);
+DEFINE_CONSTRUCTED_FLAG(int64_t);
+DEFINE_CONSTRUCTED_FLAG(uint64_t);
+DEFINE_CONSTRUCTED_FLAG(float);
+DEFINE_CONSTRUCTED_FLAG(double);
+DEFINE_CONSTRUCTED_FLAG(String);
+DEFINE_CONSTRUCTED_FLAG(UDT);
+
+template <typename T>
+bool TestConstructionFor(const flags::Flag<T>& f1, flags::Flag<T>* f2) {
+  EXPECT_EQ(f1.Name(), "f1");
+  EXPECT_EQ(f1.Help(), "literal help");
+  EXPECT_EQ(f1.Filename(), "file");
+
+  flags::FlagRegistrar<T, false>(f2).OnUpdate(TestCallback);
+
+  EXPECT_EQ(f2->Name(), "f2");
+  EXPECT_EQ(f2->Help(), "dynamic help");
+  EXPECT_EQ(f2->Filename(), "file");
+
+  return true;
+}
+
 TEST_F(FlagTest, TestConstruction) {
-  TestConstructionFor<bool>();
-  TestConstructionFor<int16_t>();
-  TestConstructionFor<uint16_t>();
-  TestConstructionFor<int32_t>();
-  TestConstructionFor<uint32_t>();
-  TestConstructionFor<int64_t>();
-  TestConstructionFor<uint64_t>();
-  TestConstructionFor<double>();
-  TestConstructionFor<float>();
-  TestConstructionFor<std::string>();
-
-  TestConstructionFor<UDT>();
+  TEST_CONSTRUCTED_FLAG(bool);
+  TEST_CONSTRUCTED_FLAG(int16_t);
+  TEST_CONSTRUCTED_FLAG(uint16_t);
+  TEST_CONSTRUCTED_FLAG(int32_t);
+  TEST_CONSTRUCTED_FLAG(uint32_t);
+  TEST_CONSTRUCTED_FLAG(int64_t);
+  TEST_CONSTRUCTED_FLAG(uint64_t);
+  TEST_CONSTRUCTED_FLAG(float);
+  TEST_CONSTRUCTED_FLAG(double);
+  TEST_CONSTRUCTED_FLAG(String);
+  TEST_CONSTRUCTED_FLAG(UDT);
 }
 
 // --------------------------------------------------------------------
@@ -391,17 +453,18 @@ TEST_F(FlagTest, TestCustomUDT) {
 using FlagDeathTest = FlagTest;
 
 TEST_F(FlagDeathTest, TestTypeMismatchValidations) {
-  EXPECT_DEBUG_DEATH(
-      static_cast<void>(absl::GetFlag(FLAGS_mistyped_int_flag)),
-      "Flag 'mistyped_int_flag' is defined as one type and declared "
-      "as another");
-  EXPECT_DEATH(absl::SetFlag(&FLAGS_mistyped_int_flag, 1),
+#if !defined(NDEBUG)
+  EXPECT_DEATH(static_cast<void>(absl::GetFlag(FLAGS_mistyped_int_flag)),
                "Flag 'mistyped_int_flag' is defined as one type and declared "
                "as another");
-
   EXPECT_DEATH(static_cast<void>(absl::GetFlag(FLAGS_mistyped_string_flag)),
                "Flag 'mistyped_string_flag' is defined as one type and "
                "declared as another");
+#endif
+
+  EXPECT_DEATH(absl::SetFlag(&FLAGS_mistyped_int_flag, 1),
+               "Flag 'mistyped_int_flag' is defined as one type and declared "
+               "as another");
   EXPECT_DEATH(
       absl::SetFlag(&FLAGS_mistyped_string_flag, std::vector<std::string>{}),
       "Flag 'mistyped_string_flag' is defined as one type and declared as "
diff --git a/absl/flags/internal/commandlineflag.cc b/absl/flags/internal/commandlineflag.cc
new file mode 100644
index 0000000000..90765a3eb6
--- /dev/null
+++ b/absl/flags/internal/commandlineflag.cc
@@ -0,0 +1,30 @@
+//
+// Copyright 2020 The Abseil Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "absl/flags/internal/commandlineflag.h"
+
+namespace absl {
+ABSL_NAMESPACE_BEGIN
+namespace flags_internal {
+
+FlagStateInterface::~FlagStateInterface() {}
+
+bool CommandLineFlag::IsRetired() const { return false; }
+bool CommandLineFlag::IsAbseilFlag() const { return true; }
+
+}  // namespace flags_internal
+ABSL_NAMESPACE_END
+}  // namespace absl
+
diff --git a/absl/flags/internal/commandlineflag.h b/absl/flags/internal/commandlineflag.h
index 6363c6615b..e91ddde633 100644
--- a/absl/flags/internal/commandlineflag.h
+++ b/absl/flags/internal/commandlineflag.h
@@ -77,7 +77,7 @@ enum ValueSource {
 // of a flag produced this flag state from method CommandLineFlag::SaveState().
 class FlagStateInterface {
  public:
-  virtual ~FlagStateInterface() {}
+  virtual ~FlagStateInterface();
 
   // Restores the flag originated this object to the saved state.
   virtual void Restore() const = 0;
@@ -146,9 +146,9 @@ class CommandLineFlag {
   // Returns help message associated with this flag.
   virtual std::string Help() const = 0;
   // Returns true iff this object corresponds to retired flag.
-  virtual bool IsRetired() const { return false; }
+  virtual bool IsRetired() const;
   // Returns true iff this is a handle to an Abseil Flag.
-  virtual bool IsAbseilFlag() const { return true; }
+  virtual bool IsAbseilFlag() const;
   // Returns id of the flag's value type.
   virtual FlagStaticTypeId TypeId() const = 0;
   virtual bool IsModified() const = 0;
diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc
index 5a921e28d7..a944e16e50 100644
--- a/absl/flags/internal/flag.cc
+++ b/absl/flags/internal/flag.cc
@@ -77,19 +77,33 @@ class MutexRelock {
 void FlagImpl::Init() {
   new (&data_guard_) absl::Mutex;
 
-  absl::MutexLock lock(reinterpret_cast<absl::Mutex*>(&data_guard_));
-
-  value_.dynamic = MakeInitValue().release();
-  StoreAtomic();
+  // At this point the default_value_ always points to gen_func.
+  std::unique_ptr<void, DynValueDeleter> init_value(
+      (*default_value_.gen_func)(), DynValueDeleter{op_});
+  switch (ValueStorageKind()) {
+    case FlagValueStorageKind::kHeapAllocated:
+      value_.dynamic = init_value.release();
+      break;
+    case FlagValueStorageKind::kOneWordAtomic: {
+      int64_t atomic_value;
+      std::memcpy(&atomic_value, init_value.get(), Sizeof(op_));
+      value_.one_word_atomic.store(atomic_value, std::memory_order_release);
+      break;
+    }
+    case FlagValueStorageKind::kTwoWordsAtomic: {
+      AlignedTwoWords atomic_value{0, 0};
+      std::memcpy(&atomic_value, init_value.get(), Sizeof(op_));
+      value_.two_words_atomic.store(atomic_value, std::memory_order_release);
+      break;
+    }
+  }
 }
 
-// Ensures that the lazily initialized data is initialized,
-// and returns pointer to the mutex guarding flags data.
 absl::Mutex* FlagImpl::DataGuard() const {
   absl::call_once(const_cast<FlagImpl*>(this)->init_control_, &FlagImpl::Init,
                   const_cast<FlagImpl*>(this));
 
-  // data_guard_ is initialized.
+  // data_guard_ is initialized inside Init.
   return reinterpret_cast<absl::Mutex*>(&data_guard_);
 }
 
@@ -129,8 +143,24 @@ std::unique_ptr<void, DynValueDeleter> FlagImpl::MakeInitValue() const {
 }
 
 void FlagImpl::StoreValue(const void* src) {
-  flags_internal::Copy(op_, src, value_.dynamic);
-  StoreAtomic();
+  switch (ValueStorageKind()) {
+    case FlagValueStorageKind::kHeapAllocated:
+      Copy(op_, src, value_.dynamic);
+      break;
+    case FlagValueStorageKind::kOneWordAtomic: {
+      int64_t one_word_val;
+      std::memcpy(&one_word_val, src, Sizeof(op_));
+      value_.one_word_atomic.store(one_word_val, std::memory_order_release);
+      break;
+    }
+    case FlagValueStorageKind::kTwoWordsAtomic: {
+      AlignedTwoWords two_words_val{0, 0};
+      std::memcpy(&two_words_val, src, Sizeof(op_));
+      value_.two_words_atomic.store(two_words_val, std::memory_order_release);
+      break;
+    }
+  }
+
   modified_ = true;
   ++counter_;
   InvokeCallback();
@@ -165,9 +195,25 @@ std::string FlagImpl::DefaultValue() const {
 }
 
 std::string FlagImpl::CurrentValue() const {
-  absl::MutexLock l(DataGuard());
+  DataGuard();  // Make sure flag initialized
+  switch (ValueStorageKind()) {
+    case FlagValueStorageKind::kHeapAllocated: {
+      absl::MutexLock l(DataGuard());
+      return flags_internal::Unparse(op_, value_.dynamic);
+    }
+    case FlagValueStorageKind::kOneWordAtomic: {
+      const auto one_word_val =
+          value_.one_word_atomic.load(std::memory_order_acquire);
+      return flags_internal::Unparse(op_, &one_word_val);
+    }
+    case FlagValueStorageKind::kTwoWordsAtomic: {
+      const auto two_words_val =
+          value_.two_words_atomic.load(std::memory_order_acquire);
+      return flags_internal::Unparse(op_, &two_words_val);
+    }
+  }
 
-  return flags_internal::Unparse(op_, value_.dynamic);
+  return "";
 }
 
 void FlagImpl::SetCallback(const FlagCallbackFunc mutation_callback) {
@@ -244,26 +290,27 @@ std::unique_ptr<void, DynValueDeleter> FlagImpl::TryParse(
 }
 
 void FlagImpl::Read(void* dst) const {
-  absl::ReaderMutexLock l(DataGuard());
+  DataGuard();  // Make sure flag initialized
+  switch (ValueStorageKind()) {
+    case FlagValueStorageKind::kHeapAllocated: {
+      absl::MutexLock l(DataGuard());
 
-  flags_internal::CopyConstruct(op_, value_.dynamic, dst);
-}
-
-void FlagImpl::StoreAtomic() {
-  size_t data_size = flags_internal::Sizeof(op_);
-
-  if (data_size <= sizeof(int64_t)) {
-    int64_t t = 0;
-    std::memcpy(&t, value_.dynamic, data_size);
-    value_.atomics.small_atomic.store(t, std::memory_order_release);
-  }
-#if defined(ABSL_FLAGS_INTERNAL_ATOMIC_DOUBLE_WORD)
-  else if (data_size <= sizeof(FlagsInternalTwoWordsType)) {
-    FlagsInternalTwoWordsType t{0, 0};
-    std::memcpy(&t, value_.dynamic, data_size);
-    value_.atomics.big_atomic.store(t, std::memory_order_release);
+      flags_internal::CopyConstruct(op_, value_.dynamic, dst);
+      break;
+    }
+    case FlagValueStorageKind::kOneWordAtomic: {
+      const auto one_word_val =
+          value_.one_word_atomic.load(std::memory_order_acquire);
+      std::memcpy(dst, &one_word_val, Sizeof(op_));
+      break;
+    }
+    case FlagValueStorageKind::kTwoWordsAtomic: {
+      const auto two_words_val =
+          value_.two_words_atomic.load(std::memory_order_acquire);
+      std::memcpy(dst, &two_words_val, Sizeof(op_));
+      break;
+    }
   }
-#endif
 }
 
 void FlagImpl::Write(const void* src) {
@@ -339,7 +386,7 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode,
       }
 
       if (!modified_) {
-        // Need to set both default value *and* current, in this case
+        // Need to set both default value *and* current, in this case.
         StoreValue(default_value_.dynamic_value);
         modified_ = false;
       }
diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h
index 35a148cf66..307b737752 100644
--- a/absl/flags/internal/flag.h
+++ b/absl/flags/internal/flag.h
@@ -31,6 +31,7 @@
 #include "absl/flags/internal/commandlineflag.h"
 #include "absl/flags/internal/registry.h"
 #include "absl/memory/memory.h"
+#include "absl/meta/type_traits.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
 #include "absl/synchronization/mutex.h"
@@ -249,95 +250,66 @@ enum class FlagDefaultKind : uint8_t { kDynamicValue = 0, kGenFunc = 1 };
 ///////////////////////////////////////////////////////////////////////////////
 // Flag current value auxiliary structs.
 
-// The minimum atomic size we believe to generate lock free code, i.e. all
-// trivially copyable types not bigger this size generate lock free code.
-static constexpr int kMinLockFreeAtomicSize = 8;
+constexpr int64_t UninitializedFlagValue() { return 0xababababababababll; }
 
-// The same as kMinLockFreeAtomicSize but maximum atomic size. As double words
-// might use two registers, we want to dispatch the logic for them.
-#if defined(ABSL_FLAGS_INTERNAL_ATOMIC_DOUBLE_WORD)
-static constexpr int kMaxLockFreeAtomicSize = 16;
-#else
-static constexpr int kMaxLockFreeAtomicSize = 8;
-#endif
-
-// We can use atomic in cases when it fits in the register, trivially copyable
-// in order to make memcpy operations.
 template <typename T>
-struct IsAtomicFlagTypeTrait {
-  static constexpr bool value =
-      (sizeof(T) <= kMaxLockFreeAtomicSize &&
-       type_traits_internal::is_trivially_copyable<T>::value);
-};
+using FlagUseOneWordStorage = std::integral_constant<
+    bool, absl::type_traits_internal::is_trivially_copyable<T>::value &&
+              (sizeof(T) <= 8)>;
 
+#if defined(ABSL_FLAGS_INTERNAL_ATOMIC_DOUBLE_WORD)
 // Clang does not always produce cmpxchg16b instruction when alignment of a 16
 // bytes type is not 16.
-struct alignas(16) FlagsInternalTwoWordsType {
+struct alignas(16) AlignedTwoWords {
   int64_t first;
   int64_t second;
 };
 
-constexpr bool operator==(const FlagsInternalTwoWordsType& that,
-                          const FlagsInternalTwoWordsType& other) {
-  return that.first == other.first && that.second == other.second;
-}
-constexpr bool operator!=(const FlagsInternalTwoWordsType& that,
-                          const FlagsInternalTwoWordsType& other) {
-  return !(that == other);
-}
-
-constexpr int64_t SmallAtomicInit() { return 0xababababababababll; }
-
-template <typename T, typename S = void>
-struct BestAtomicType {
-  using type = int64_t;
-  static constexpr int64_t AtomicInit() { return SmallAtomicInit(); }
+template <typename T>
+using FlagUseTwoWordsStorage = std::integral_constant<
+    bool, absl::type_traits_internal::is_trivially_copyable<T>::value &&
+              (sizeof(T) > 8) && (sizeof(T) <= 16)>;
+#else
+// This is actually unused and only here to avoid ifdefs in other palces.
+struct AlignedTwoWords {
+  constexpr AlignedTwoWords() = default;
+  constexpr AlignedTwoWords(int64_t, int64_t) {}
 };
 
+// This trait should be type dependent, otherwise SFINAE below will fail
 template <typename T>
-struct BestAtomicType<
-    T, typename std::enable_if<(kMinLockFreeAtomicSize < sizeof(T) &&
-                                sizeof(T) <= kMaxLockFreeAtomicSize),
-                               void>::type> {
-  using type = FlagsInternalTwoWordsType;
-  static constexpr FlagsInternalTwoWordsType AtomicInit() {
-    return {SmallAtomicInit(), SmallAtomicInit()};
-  }
+using FlagUseTwoWordsStorage =
+    std::integral_constant<bool, sizeof(T) != sizeof(T)>;
+#endif
+
+template <typename T>
+using FlagUseHeapStorage =
+    std::integral_constant<bool, !FlagUseOneWordStorage<T>::value &&
+                                     !FlagUseTwoWordsStorage<T>::value>;
+
+enum class FlagValueStorageKind : uint8_t {
+  kHeapAllocated = 0,
+  kOneWordAtomic = 1,
+  kTwoWordsAtomic = 2
 };
 
-struct FlagValue {
-  // Heap allocated value.
-  void* dynamic = nullptr;
-  // For some types, a copy of the current value is kept in an atomically
-  // accessible field.
-  union Atomics {
-    // Using small atomic for small types.
-    std::atomic<int64_t> small_atomic;
-    template <typename T,
-              typename K = typename std::enable_if<
-                  (sizeof(T) <= kMinLockFreeAtomicSize), void>::type>
-    int64_t load() const {
-      return small_atomic.load(std::memory_order_acquire);
-    }
+union FlagValue {
+  constexpr explicit FlagValue(int64_t v) : one_word_atomic(v) {}
 
-#if defined(ABSL_FLAGS_INTERNAL_ATOMIC_DOUBLE_WORD)
-    // Using big atomics for big types.
-    std::atomic<FlagsInternalTwoWordsType> big_atomic;
-    template <typename T, typename K = typename std::enable_if<
-                              (kMinLockFreeAtomicSize < sizeof(T) &&
-                               sizeof(T) <= kMaxLockFreeAtomicSize),
-                              void>::type>
-    FlagsInternalTwoWordsType load() const {
-      return big_atomic.load(std::memory_order_acquire);
-    }
-    constexpr Atomics()
-        : big_atomic{FlagsInternalTwoWordsType{SmallAtomicInit(),
-                                               SmallAtomicInit()}} {}
-#else
-    constexpr Atomics() : small_atomic{SmallAtomicInit()} {}
-#endif
-  };
-  Atomics atomics{};
+  template <typename T>
+  static constexpr FlagValueStorageKind Kind() {
+    return FlagUseHeapStorage<T>::value
+               ? FlagValueStorageKind::kHeapAllocated
+               : FlagUseOneWordStorage<T>::value
+                     ? FlagValueStorageKind::kOneWordAtomic
+                     : FlagUseTwoWordsStorage<T>::value
+                           ? FlagValueStorageKind::kTwoWordsAtomic
+                           : FlagValueStorageKind::kHeapAllocated;
+  }
+
+  void* dynamic;
+  std::atomic<int64_t> one_word_atomic;
+  std::atomic<flags_internal::AlignedTwoWords> two_words_atomic;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -369,18 +341,21 @@ struct DynValueDeleter {
 class FlagImpl {
  public:
   constexpr FlagImpl(const char* name, const char* filename, FlagOpFn op,
-                     FlagHelpArg help, FlagDfltGenFunc default_value_gen)
+                     FlagHelpArg help, FlagValueStorageKind value_kind,
+                     FlagDfltGenFunc default_value_gen)
       : name_(name),
         filename_(filename),
         op_(op),
         help_(help.source),
         help_source_kind_(static_cast<uint8_t>(help.kind)),
+        value_storage_kind_(static_cast<uint8_t>(value_kind)),
         def_kind_(static_cast<uint8_t>(FlagDefaultKind::kGenFunc)),
         modified_(false),
         on_command_line_(false),
         counter_(0),
         callback_(nullptr),
         default_value_(default_value_gen),
+        value_(flags_internal::UninitializedFlagValue()),
         data_guard_{} {}
 
   // Constant access methods
@@ -393,34 +368,29 @@ class FlagImpl {
   std::string CurrentValue() const ABSL_LOCKS_EXCLUDED(*DataGuard());
   void Read(void* dst) const ABSL_LOCKS_EXCLUDED(*DataGuard());
 
-  template <typename T, typename std::enable_if<
-                            !IsAtomicFlagTypeTrait<T>::value, int>::type = 0>
+  template <typename T, typename std::enable_if<FlagUseHeapStorage<T>::value,
+                                                int>::type = 0>
   void Get(T* dst) const {
-    AssertValidType(&flags_internal::FlagStaticTypeIdGen<T>);
     Read(dst);
   }
-  // Overload for `GetFlag()` for types that support lock-free reads.
-  template <typename T, typename std::enable_if<IsAtomicFlagTypeTrait<T>::value,
+  template <typename T, typename std::enable_if<FlagUseOneWordStorage<T>::value,
                                                 int>::type = 0>
   void Get(T* dst) const {
-    // For flags of types which can be accessed "atomically" we want to avoid
-    // slowing down flag value access due to type validation. That's why
-    // this validation is hidden behind !NDEBUG
-#ifndef NDEBUG
-    AssertValidType(&flags_internal::FlagStaticTypeIdGen<T>);
-#endif
-    using U = flags_internal::BestAtomicType<T>;
-    typename U::type r = value_.atomics.template load<T>();
-    if (r != U::AtomicInit()) {
-      std::memcpy(static_cast<void*>(dst), &r, sizeof(T));
-    } else {
-      Read(dst);
+    int64_t one_word_val =
+        value_.one_word_atomic.load(std::memory_order_acquire);
+    if (ABSL_PREDICT_FALSE(one_word_val == UninitializedFlagValue())) {
+      DataGuard();  // Make sure flag initialized
+      one_word_val = value_.one_word_atomic.load(std::memory_order_acquire);
     }
+    std::memcpy(dst, static_cast<const void*>(&one_word_val), sizeof(T));
   }
-  template <typename T>
-  void Set(const T& src) {
-    AssertValidType(&flags_internal::FlagStaticTypeIdGen<T>);
-    Write(&src);
+  template <typename T, typename std::enable_if<
+                            FlagUseTwoWordsStorage<T>::value, int>::type = 0>
+  void Get(T* dst) const {
+    DataGuard();  // Make sure flag initialized
+    const auto two_words_val =
+        value_.two_words_atomic.load(std::memory_order_acquire);
+    std::memcpy(dst, &two_words_val, sizeof(T));
   }
 
   // Mutating access methods
@@ -428,9 +398,6 @@ class FlagImpl {
   bool SetFromString(absl::string_view value, FlagSettingMode set_mode,
                      ValueSource source, std::string* err)
       ABSL_LOCKS_EXCLUDED(*DataGuard());
-  // If possible, updates copy of the Flag's value that is stored in an
-  // atomic word.
-  void StoreAtomic() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
 
   // Interfaces to operate on callbacks.
   void SetCallback(const FlagCallbackFunc mutation_callback)
@@ -456,6 +423,14 @@ class FlagImpl {
   bool ValidateInputValue(absl::string_view value) const
       ABSL_LOCKS_EXCLUDED(*DataGuard());
 
+  // Used in read/write operations to validate source/target has correct type.
+  // For example if flag is declared as absl::Flag<int> FLAGS_foo, a call to
+  // absl::GetFlag(FLAGS_foo) validates that the type of FLAGS_foo is indeed
+  // int. To do that we pass the "assumed" type id (which is deduced from type
+  // int) as an argument `op`, which is in turn is validated against the type id
+  // stored in flag object by flag definition statement.
+  void AssertValidType(FlagStaticTypeId type_id) const;
+
  private:
   // Ensures that `data_guard_` is initialized and returns it.
   absl::Mutex* DataGuard() const ABSL_LOCK_RETURNED((absl::Mutex*)&data_guard_);
@@ -475,17 +450,13 @@ class FlagImpl {
   FlagHelpKind HelpSourceKind() const {
     return static_cast<FlagHelpKind>(help_source_kind_);
   }
+  FlagValueStorageKind ValueStorageKind() const {
+    return static_cast<FlagValueStorageKind>(value_storage_kind_);
+  }
   FlagDefaultKind DefaultKind() const
       ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard()) {
     return static_cast<FlagDefaultKind>(def_kind_);
   }
-  // Used in read/write operations to validate source/target has correct type.
-  // For example if flag is declared as absl::Flag<int> FLAGS_foo, a call to
-  // absl::GetFlag(FLAGS_foo) validates that the type of FLAGS_foo is indeed
-  // int. To do that we pass the "assumed" type id (which is deduced from type
-  // int) as an argument `op`, which is in turn is validated against the type id
-  // stored in flag object by flag definition statement.
-  void AssertValidType(FlagStaticTypeId type_id) const;
 
   // Immutable flag's state.
 
@@ -499,6 +470,8 @@ class FlagImpl {
   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;
 
   // ------------------------------------------------------------------------
   // The bytes containing the const bitfields must not be shared with bytes
@@ -530,8 +503,13 @@ class FlagImpl {
   // 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_ ABSL_GUARDED_BY(*DataGuard());
-  // Current Flag Value
+  FlagDefaultSrc default_value_;
+
+  // Atomically mutable flag's state
+
+  // Flag's value. This can be either the atomically stored small value or
+  // pointer to the heap allocated dynamic value. value_storage_kind_ is used
+  // to distinguish these cases.
   FlagValue value_;
 
   // This is reserved space for an absl::Mutex to guard flag data. It will be
@@ -553,7 +531,8 @@ class Flag final : public flags_internal::CommandLineFlag {
  public:
   constexpr Flag(const char* name, const char* filename, const FlagHelpArg help,
                  const FlagDfltGenFunc default_value_gen)
-      : impl_(name, filename, &FlagOps<T>, help, default_value_gen) {}
+      : impl_(name, filename, &FlagOps<T>, help, FlagValue::Kind<T>(),
+              default_value_gen) {}
 
   T Get() const {
     // See implementation notes in CommandLineFlag::Get().
@@ -564,10 +543,17 @@ class Flag final : public flags_internal::CommandLineFlag {
     };
     U u;
 
+#if !defined(NDEBUG)
+    impl_.AssertValidType(&flags_internal::FlagStaticTypeIdGen<T>);
+#endif
+
     impl_.Get(&u.value);
     return std::move(u.value);
   }
-  void Set(const T& v) { impl_.Set(v); }
+  void Set(const T& v) {
+    impl_.AssertValidType(&flags_internal::FlagStaticTypeIdGen<T>);
+    impl_.Write(&v);
+  }
   void SetCallback(const FlagCallbackFunc mutation_callback) {
     impl_.SetCallback(mutation_callback);
   }
@@ -619,7 +605,7 @@ class Flag final : public flags_internal::CommandLineFlag {
 };
 
 template <typename T>
-inline void FlagState<T>::Restore() const {
+void FlagState<T>::Restore() const {
   if (flag_->RestoreState(*this)) {
     ABSL_INTERNAL_LOG(INFO,
                       absl::StrCat("Restore saved value of ", flag_->Name(),