[counters] Always provide a backing pointer

This avoids the {StatsCounter::lookup_done_} field by always
initializing the {StatsCounter::ptr_} field in {StatsCounter::GetPtr()}.
This makes the fast path for updating the counter value much simpler and
faster.

R=mlippautz@chromium.org

Bug: v8:12482
Change-Id: I89d094b15e0417bbfb302006de8eede0c200202d
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_ubsan_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3322768
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78314}

src/logging/counters.cc

@@ -24,8 +24,26 @@ void StatsTable::SetCounterFunction(CounterLookupCallback f) {
   lookup_function_ = f;
 }
 
-int* StatsCounter::FindLocationInStatsTable() const {
-  return counters_->FindLocation(name_);
+namespace {
+std::atomic<int> unused_counter_dump{0};
+}
+
+bool StatsCounter::Enabled() { return GetPtr() != &unused_counter_dump; }
+
+std::atomic<int>* StatsCounter::SetupPtrFromStatsTable() {
+  // {Init} must have been called.
+  DCHECK_NOT_NULL(counters_);
+  DCHECK_NOT_NULL(name_);
+  int* location = counters_->FindLocation(name_);
+  std::atomic<int>* ptr =
+      location ? base::AsAtomicPtr(location) : &unused_counter_dump;
+#ifdef DEBUG
+  std::atomic<int>* old_ptr = ptr_.exchange(ptr, std::memory_order_release);
+  DCHECK_IMPLIES(old_ptr, old_ptr == ptr);
+#else
+  ptr_.store(ptr, std::memory_order_release);
+#endif
+  return ptr;
 }
 
 void Histogram::AddSample(int sample) {

src/logging/counters.h

@@ -101,36 +101,24 @@ class StatsTable {
 // This class is thread-safe.
 class StatsCounter {
  public:
-  void Set(int value) {
-    if (std::atomic<int>* loc = GetPtr()) {
-      loc->store(value, std::memory_order_relaxed);
-    }
-  }
+  void Set(int value) { GetPtr()->store(value, std::memory_order_relaxed); }
 
   void Increment(int value = 1) {
-    if (std::atomic<int>* loc = GetPtr()) {
-      loc->fetch_add(value, std::memory_order_relaxed);
-    }
+    GetPtr()->fetch_add(value, std::memory_order_relaxed);
   }
 
   void Decrement(int value = 1) {
-    if (std::atomic<int>* loc = GetPtr()) {
-      loc->fetch_sub(value, std::memory_order_relaxed);
-    }
+    GetPtr()->fetch_sub(value, std::memory_order_relaxed);
   }
 
-  // Is this counter enabled?
-  // Returns false if table is full.
-  bool Enabled() { return GetPtr() != nullptr; }
+  // Returns true if this counter is enabled (a lookup function was provided and
+  // it returned a non-null pointer).
+  V8_EXPORT_PRIVATE bool Enabled();
 
   // Get the internal pointer to the counter. This is used
   // by the code generator to emit code that manipulates a
   // given counter without calling the runtime system.
-  std::atomic<int>* GetInternalPointer() {
-    std::atomic<int>* loc = GetPtr();
-    DCHECK_NOT_NULL(loc);
-    return loc;
-  }
+  std::atomic<int>* GetInternalPointer() { return GetPtr(); }
 
  private:
   friend class Counters;
@@ -144,35 +132,22 @@ class StatsCounter {
     name_ = name;
   }
 
-  V8_EXPORT_PRIVATE int* FindLocationInStatsTable() const;
+  V8_NOINLINE V8_EXPORT_PRIVATE std::atomic<int>* SetupPtrFromStatsTable();
 
   // Reset the cached internal pointer.
-  void Reset() {
-    lookup_done_.store(false, std::memory_order_release);
-    ptr_.store(nullptr, std::memory_order_release);
-  }
+  void Reset() { ptr_.store(nullptr, std::memory_order_relaxed); }
 
   // Returns the cached address of this counter location.
   std::atomic<int>* GetPtr() {
-    // {Init} must have been called.
-    DCHECK_NOT_NULL(counters_);
-    DCHECK_NOT_NULL(name_);
     auto* ptr = ptr_.load(std::memory_order_acquire);
     if (V8_LIKELY(ptr)) return ptr;
-    if (!lookup_done_.load(std::memory_order_acquire)) {
-      ptr = base::AsAtomicPtr(FindLocationInStatsTable());
-      ptr_.store(ptr, std::memory_order_release);
-      lookup_done_.store(true, std::memory_order_release);
-    }
-    // Re-load after checking {lookup_done_}.
-    return ptr_.load(std::memory_order_acquire);
+    return SetupPtrFromStatsTable();
   }
 
   Counters* counters_ = nullptr;
   const char* name_ = nullptr;
   // A pointer to an atomic, set atomically in {GetPtr}.
   std::atomic<std::atomic<int>*> ptr_{nullptr};
-  std::atomic<bool> lookup_done_{false};
 };
 
 // A Histogram represents a dynamically created histogram in the