[PATCH] D27724: Statistic: Initialize in the constructor

[Matthias Braun via Phabricator via llvm-commits]

MatzeB created this revision.
MatzeB added reviewers: chandlerc, dblaikie, echristo.
MatzeB added a subscriber: llvm-commits.
MatzeB set the repository for this revision to rL LLVM.
Herald added subscribers: mehdi_amini, mcrosier.

Statistics should be fast enough that we can enable them in release builds without compiletime impact. This does not work today because the lazy initialization in use today puts a memory fence on every increment operation!

The obvious solution for this is initializing the statistic variable in the constructor. Which is easy to do and works better in every way than todays code IMO except that introduce additional static constructors into llvm which in this specific instance look fine to me; the alternative of forbidding global variables for statistic variables appears arguably worse.


Repository:
  rL LLVM

https://reviews.llvm.org/D27724

Files:
  include/llvm/ADT/Statistic.h


Index: include/llvm/ADT/Statistic.h
===================================================================
--- include/llvm/ADT/Statistic.h
+++ include/llvm/ADT/Statistic.h
@@ -42,7 +42,7 @@
   const char *Name;
   const char *Desc;
   std::atomic<unsigned> Value;
-  bool Initialized;
+  bool Initialized = false;
 
   unsigned getValue() const { return Value.load(std::memory_order_relaxed); }
   const char *getDebugType() const { return DebugType; }
@@ -62,46 +62,51 @@
   operator unsigned() const { return getValue(); }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_STATS)
-   const Statistic &operator=(unsigned Val) {
+  Statistic(const char *DebugType, const char *Name, const char *Desc)
+      : DebugType(DebugType), Name(Name), Desc(Desc) {
+    init();
+  }
+
+  const Statistic &operator=(unsigned Val) {
     Value.store(Val, std::memory_order_relaxed);
-    return init();
+    return *this;
   }
 
   const Statistic &operator++() {
     Value.fetch_add(1, std::memory_order_relaxed);
-    return init();
+    return *this;
   }
 
   unsigned operator++(int) {
-    init();
     return Value.fetch_add(1, std::memory_order_relaxed);
   }
 
   const Statistic &operator--() {
     Value.fetch_sub(1, std::memory_order_relaxed);
-    return init();
+    return *this;
   }
 
   unsigned operator--(int) {
-    init();
     return Value.fetch_sub(1, std::memory_order_relaxed);
   }
 
   const Statistic &operator+=(unsigned V) {
     if (V == 0)
       return *this;
     Value.fetch_add(V, std::memory_order_relaxed);
-    return init();
+    return *this;
   }
 
   const Statistic &operator-=(unsigned V) {
     if (V == 0)
       return *this;
     Value.fetch_sub(V, std::memory_order_relaxed);
-    return init();
+    return *this;
   }
 
 #else  // Statistics are disabled in release builds.
+  Statistic(const char *, const char *, const char *)
+      : DebugType(nullptr), Name(nullptr), Desc(nullptr) {}
 
   const Statistic &operator=(unsigned Val) {
     return *this;
@@ -148,7 +153,7 @@
 // STATISTIC - A macro to make definition of statistics really simple.  This
 // automatically passes the DEBUG_TYPE of the file into the statistic.
 #define STATISTIC(VARNAME, DESC)                                               \
-  static llvm::Statistic VARNAME = {DEBUG_TYPE, #VARNAME, DESC, {0}, false}
+  static llvm::Statistic VARNAME = {DEBUG_TYPE, #VARNAME, DESC}
 
 /// \brief Enable the collection and printing of statistics.
 void EnableStatistics(bool PrintOnExit = true);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27724.81267.patch
Type: text/x-patch
Size: 2504 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161213/618c65c7/attachment.bin>