This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new a0aa9cf36 [codegen] use std::atomic instead of AtomicInt
a0aa9cf36 is described below

commit a0aa9cf363ec47ba643a56c252660d2506b32411
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Sun Mar 31 09:35:11 2024 -0700

    [codegen] use std::atomic instead of AtomicInt
    
    The chromium-based atomics are deprecated in favor of std::atomic
    from the STL: this patch switches from AtomicInt to std::atomic
    for the codegen's compilation manager.  I changes the corresponding
    metrics from int64_t to uint64_t type.
    
    This patch doesn't contain any functional modifications.
    
    Change-Id: I3271ef420163a6d996645bb60aa8d475c7925f92
    Reviewed-on: http://gerrit.cloudera.org:8080/21225
    Reviewed-by: Yingchun Lai <laiyingc...@apache.org>
    Tested-by: Alexey Serbin <ale...@apache.org>
---
 src/kudu/codegen/compilation_manager.cc | 52 +++++++++++++++++++--------------
 src/kudu/codegen/compilation_manager.h  | 13 ++++-----
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/src/kudu/codegen/compilation_manager.cc 
b/src/kudu/codegen/compilation_manager.cc
index 191263c73..9dc2d2b05 100644
--- a/src/kudu/codegen/compilation_manager.cc
+++ b/src/kudu/codegen/compilation_manager.cc
@@ -22,6 +22,7 @@
 #include <memory>
 #include <ostream>
 #include <string>
+#include <type_traits>
 
 #include <gflags/gflags.h>
 #include <glog/logging.h>
@@ -32,6 +33,7 @@
 #include "kudu/codegen/row_projector.h"
 #include "kudu/common/schema.h"
 #include "kudu/gutil/casts.h"
+#include "kudu/gutil/port.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/util/faststring.h"
 #include "kudu/util/flag_tags.h"
@@ -60,17 +62,17 @@ DEFINE_int32(codegen_queue_capacity, 100, "Number of tasks 
which may be put in t
              "generation task queue.");
 TAG_FLAG(codegen_queue_capacity, experimental);
 
-METRIC_DEFINE_gauge_int64(server, code_cache_hits, "Codegen Cache Hits",
-                          kudu::MetricUnit::kCacheHits,
-                          "Number of codegen cache hits since start",
-                          kudu::MetricLevel::kDebug,
-                          kudu::EXPOSE_AS_COUNTER);
-METRIC_DEFINE_gauge_int64(server, code_cache_queries, "Codegen Cache Queries",
-                          kudu::MetricUnit::kCacheQueries,
-                          "Number of codegen cache queries (hits + misses) "
-                          "since start",
-                          kudu::MetricLevel::kDebug,
-                          kudu::EXPOSE_AS_COUNTER);
+METRIC_DEFINE_gauge_uint64(server, code_cache_hits, "Codegen Cache Hits",
+                           kudu::MetricUnit::kCacheHits,
+                           "Number of codegen cache hits since start",
+                           kudu::MetricLevel::kDebug,
+                           kudu::EXPOSE_AS_COUNTER);
+METRIC_DEFINE_gauge_uint64(server, code_cache_queries, "Codegen Cache Queries",
+                           kudu::MetricUnit::kCacheQueries,
+                           "Number of codegen cache queries (hits + misses) "
+                           "since start",
+                           kudu::MetricLevel::kDebug,
+                           kudu::EXPOSE_AS_COUNTER);
 namespace kudu {
 namespace codegen {
 
@@ -138,7 +140,7 @@ CompilationManager::CompilationManager()
            .set_min_threads(0)
            .set_max_threads(1)
            .set_max_queue_size(FLAGS_codegen_queue_capacity)
-           .set_idle_timeout(MonoDelta::FromMilliseconds(kThreadTimeoutMs))
+           .set_idle_timeout(MonoDelta::FromMilliseconds(100))
            .Build(&pool_));
   // We call std::atexit after the implicit default construction of
   // generator_ to ensure static LLVM constants would not have been destructed
@@ -166,12 +168,16 @@ Status CompilationManager::StartInstrumentation(const 
scoped_refptr<MetricEntity
   // register the same metric in multiple registries. Using a gauge which loads
   // an atomic int is a suitable workaround: each TS's registry ends up with a
   // unique gauge which reads the value of the singleton's integer.
-  metric_entity->NeverRetire(
-      METRIC_code_cache_hits.InstantiateFunctionGauge(
-          metric_entity, [this]() { return 
this->hit_counter_.Load(kMemOrderNoBarrier); }));
-  metric_entity->NeverRetire(
-      METRIC_code_cache_queries.InstantiateFunctionGauge(
-          metric_entity, [this]() { return 
this->query_counter_.Load(kMemOrderNoBarrier); }));
+  metric_entity->NeverRetire(METRIC_code_cache_hits.InstantiateFunctionGauge(
+      metric_entity,
+      [this]() {
+        return 
this->hit_counter_.load(std::memory_order::memory_order_relaxed);
+      }));
+  
metric_entity->NeverRetire(METRIC_code_cache_queries.InstantiateFunctionGauge(
+      metric_entity,
+      [this]() {
+        return 
this->query_counter_.load(std::memory_order::memory_order_relaxed);
+      }));
   return Status::OK();
 }
 
@@ -179,10 +185,12 @@ bool CompilationManager::RequestRowProjector(const 
Schema* base_schema,
                                              const Schema* projection,
                                              unique_ptr<RowProjector>* out) {
   faststring key;
-  Status s = RowProjectorFunctions::EncodeKey(*base_schema, *projection, &key);
+  const auto s = RowProjectorFunctions::EncodeKey(*base_schema, *projection, 
&key);
   WARN_NOT_OK(s, "RowProjector compilation request encode key failed");
-  if (!s.ok()) return false;
-  query_counter_.Increment();
+  if (PREDICT_FALSE(!s.ok())) {
+    return false;
+  }
+  ++query_counter_;
 
   scoped_refptr<RowProjectorFunctions> cached(
     down_cast<RowProjectorFunctions*>(cache_.Lookup(key).get()));
@@ -196,7 +204,7 @@ bool CompilationManager::RequestRowProjector(const Schema* 
base_schema,
     return false;
   }
 
-  hit_counter_.Increment();
+  ++hit_counter_;
 
   out->reset(new RowProjector(base_schema, projection, cached));
   return true;
diff --git a/src/kudu/codegen/compilation_manager.h 
b/src/kudu/codegen/compilation_manager.h
index 7aa05dfe2..a4ddd7861 100644
--- a/src/kudu/codegen/compilation_manager.h
+++ b/src/kudu/codegen/compilation_manager.h
@@ -16,6 +16,7 @@
 // under the License.
 #pragma once
 
+#include <atomic>
 #include <cstdint>
 #include <memory>
 
@@ -24,7 +25,6 @@
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/gutil/singleton.h"
-#include "kudu/util/atomic.h"
 #include "kudu/util/status.h"
 
 namespace kudu {
@@ -56,7 +56,7 @@ class RowProjector;
 // unnecessary dependencies on state and lifetime of top-level and
 // intermediary classes which should not be aware of the code generation's
 // use in the first place.
-class CompilationManager {
+class CompilationManager final {
  public:
   // Waits for all async tasks to finish.
   ~CompilationManager();
@@ -87,18 +87,17 @@ class CompilationManager {
 
  private:
   friend class Singleton<CompilationManager>;
-  CompilationManager();
 
   static void Shutdown();
 
+  CompilationManager();
+
   CodeGenerator generator_;
   CodeCache cache_;
   std::unique_ptr<ThreadPool> pool_;
 
-  AtomicInt<int64_t> hit_counter_;
-  AtomicInt<int64_t> query_counter_;
-
-  static const int kThreadTimeoutMs = 100;
+  std::atomic<uint64_t> hit_counter_;
+  std::atomic<uint64_t> query_counter_;
 
   DISALLOW_COPY_AND_ASSIGN(CompilationManager);
 };

Reply via email to