about summary refs log tree commit diff
path: root/absl/base/internal/periodic_sampler.h
diff options
context:
space:
mode:
authorAbseil Team <absl-team@google.com>2019-11-18T19·02-0800
committerGennadiy Civil <misterg@google.com>2019-11-19T15·27-0500
commit2103fd9acdf58279f739860bff3f8c9f4b845605 (patch)
tree8eee8ff910c004b517eb02c79feac753fe19126e /absl/base/internal/periodic_sampler.h
parent3df7b52a6ada51a72a23796b844549a7b282f1b8 (diff)
Export of internal Abseil changes
--
d447fdcb801036cf08197eece193a5a706661120 by Gennadiy Rozental <rogeeff@google.com>:

Eliminate the need for static function holding help message. This decreases the cost of ABSL_FLAG abstraction by 120 bytes under clang.

PiperOrigin-RevId: 281107806

--
0aa6b91189f0e8b2381438c33465673a7ae02487 by Derek Mauro <dmauro@google.com>:

Disable the weak symbol CCTZ extension in the time test_util
on MinGW, which does not support it.

PiperOrigin-RevId: 280719769

--
67322c41c3e776eb541de90fa4526bdb49422eb6 by Abseil Team <absl-team@google.com>:

Tune PeriodicSampler implementation (for internal-use only)

PiperOrigin-RevId: 280708943

--
3a48c346340c7ed03816645cd327e1ff07729aa4 by Abseil Team <absl-team@google.com>:

Clean up public headers not to have warnings for "-Wcomma"

PiperOrigin-RevId: 280695373

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

Release absl::int128.

PiperOrigin-RevId: 280690817

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

Fix -Wundef warnings in random platform detection

PiperOrigin-RevId: 280669598
GitOrigin-RevId: d447fdcb801036cf08197eece193a5a706661120
Change-Id: Ie5e10e567c54b7de211833607689f233d4ddf734
Diffstat (limited to 'absl/base/internal/periodic_sampler.h')
-rw-r--r--absl/base/internal/periodic_sampler.h62
1 files changed, 39 insertions, 23 deletions
diff --git a/absl/base/internal/periodic_sampler.h b/absl/base/internal/periodic_sampler.h
index 2c0600f05ea4..238797424fc0 100644
--- a/absl/base/internal/periodic_sampler.h
+++ b/absl/base/internal/periodic_sampler.h
@@ -111,33 +111,49 @@ class PeriodicSamplerBase {
   // Returns the current period of this sampler. Thread-safe.
   virtual int period() const noexcept = 0;
 
-  int64_t stride_ = 0;
+  // Keep and decrement stride_ as an unsigned integer, but compare the value
+  // to zero casted as a signed int. clang and msvc do not create optimum code
+  // if we use signed for the combined decrement and sign comparison.
+  //
+  // Below 3 alternative options, all compiles generate the best code
+  // using the unsigned increment <---> signed int comparison option.
+  //
+  // Option 1:
+  //   int64_t stride_;
+  //   if (ABSL_PREDICT_TRUE(++stride_ < 0)) { ... }
+  //
+  //   GCC   x64 (OK) : https://gcc.godbolt.org/z/R5MzzA
+  //   GCC   ppc (OK) : https://gcc.godbolt.org/z/z7NZAt
+  //   Clang x64 (BAD): https://gcc.godbolt.org/z/t4gPsd
+  //   ICC   x64 (OK) : https://gcc.godbolt.org/z/rE6s8W
+  //   MSVC  x64 (OK) : https://gcc.godbolt.org/z/ARMXqS
+  //
+  // Option 2:
+  //   int64_t stride_ = 0;
+  //   if (ABSL_PREDICT_TRUE(--stride_ >= 0)) { ... }
+  //
+  //   GCC   x64 (OK) : https://gcc.godbolt.org/z/jSQxYK
+  //   GCC   ppc (OK) : https://gcc.godbolt.org/z/VJdYaA
+  //   Clang x64 (BAD): https://gcc.godbolt.org/z/Xm4NjX
+  //   ICC   x64 (OK) : https://gcc.godbolt.org/z/4snaFd
+  //   MSVC  x64 (BAD): https://gcc.godbolt.org/z/BgnEKE
+  //
+  // Option 3:
+  //   uint64_t stride_;
+  //   if (ABSL_PREDICT_TRUE(static_cast<int64_t>(++stride_) < 0)) { ... }
+  //
+  //   GCC   x64 (OK) : https://gcc.godbolt.org/z/bFbfPy
+  //   GCC   ppc (OK) : https://gcc.godbolt.org/z/S9KkUE
+  //   Clang x64 (OK) : https://gcc.godbolt.org/z/UYzRb4
+  //   ICC   x64 (OK) : https://gcc.godbolt.org/z/ptTNfD
+  //   MSVC  x64 (OK) : https://gcc.godbolt.org/z/76j4-5
+  uint64_t stride_ = 0;
   ExponentialBiased rng_;
 };
 
 inline bool PeriodicSamplerBase::SubtleMaybeSample() noexcept {
-  // We explicitly count up and not down, as the compiler does not generate
-  // ideal code for counting down. See also https://gcc.godbolt.org/z/FTPDSM
-  //
-  // With `if (ABSL_PREDICT_FALSE(++stride_ < 0))`
-  //    add     QWORD PTR fs:sampler@tpoff+8, 1
-  //    jns     .L15
-  //    ret
-  //
-  // With `if (ABSL_PREDICT_FALSE(--stride_ > 0))`
-  //    mov     rax, QWORD PTR fs:sampler@tpoff+8
-  //    sub     rax, 1
-  //    mov     QWORD PTR fs:sampler@tpoff+8, rax
-  //    test    rax, rax
-  //    jle     .L15
-  //    ret
-  //    add     QWORD PTR fs:sampler@tpoff+8, 1
-  //    jns     .L15
-  //    ret
-  //
-  // --stride >= 0 does work, but makes our logic slightly harder as in that
-  // case we have less convenient zero-init and over-run values.
-  if (ABSL_PREDICT_FALSE(++stride_ < 0)) {
+  // See comments on `stride_` for the unsigned increment / signed compare.
+  if (ABSL_PREDICT_TRUE(static_cast<int64_t>(++stride_) < 0)) {
     return false;
   }
   return true;