[PATCH] D27724: Statistic: Initialize in the constructor

[David Blaikie via llvm-commits]

On Tue, Dec 13, 2016 at 11:41 AM Matthias Braun via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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


Tihs is a fairly big 'except'. LLVM is (reasonably) pretty averse to global
ctors - due to its use as a library, global ctors place a strong burden on
our users.

I'm not sure we'd want to add more, probably not.


> 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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161213/9ddd1300/attachment.html>