Repository: google/benchmark
File: src/perf_counters.cc (C++)

Code before:

PerfCounters PerfCounters::Create(
    const std::vector<std::string>& counter_names) {
  if (!counter_names.empty()) {
    GetErrorLogInstance() << "Performance counters not supported.";
  }
  return NoCounters();
}

PerfCounters::~PerfCounters() = default;
#endif  // defined HAVE_LIBPFM

int PerfCountersMeasurement::ref_count_ = 0;
std::unique_ptr<PerfCounters> PerfCountersMeasurement::counters_;

PerfCountersMeasurement::PerfCountersMeasurement(
    const std::vector<std::string>& counter_names)
    : start_values_(counter_names.size()),
      end_values_(counter_names.size()) {
  {
    // TODO: Add MutexLock
    //MutexLock l(mutex_);
    if (!counters_) {
      PerfCounters perf_counters = PerfCounters::Create(counter_names);
      counters_ = std::unique_ptr<PerfCounters> (
          new PerfCounters(std::move(perf_counters)));
    }
  }

  BM_CHECK(counters_->names() == counter_names);
  ++ref_count_;
}

PerfCountersMeasurement::~PerfCountersMeasurement() {
  --ref_count_;
  if (ref_count_ == 0) {
    counters_.reset();
  }
}
}  // namespace internal
}  // namespace benchmark


Review comment (suggestion):
This is can be simplified if PerfCounters::Create returns a std::unique_ptr; OR come to think of it, I think we wanted PerfCounters to be allocatable inlined (i.e. on the stack, or as a field of a class/struct), so we'd avoid a dereference and added noise to measurements.

So let's have `counters_` be a PerfCounters object, invalid initially (add a default ctor for `PerfCounters` to do that) and a `reset()` function on it that does what the dtor does today (the dtor would call it, too, for completeness). The design is logically the same.

Code after:

PerfCounters PerfCounters::Create(
    const std::vector<std::string>& counter_names) {
  if (!counter_names.empty()) {
    GetErrorLogInstance() << "Performance counters not supported.";
  }
  return NoCounters();
}

void PerfCounters::CloseCounters() const {}
#endif  // defined HAVE_LIBPFM

Mutex PerfCountersMeasurement::mutex_;
int PerfCountersMeasurement::ref_count_ = 0;
PerfCounters PerfCountersMeasurement::counters_ = PerfCounters::NoCounters();

PerfCountersMeasurement::PerfCountersMeasurement(
    const std::vector<std::string>& counter_names)
    : start_values_(counter_names.size()), end_values_(counter_names.size()) {
  MutexLock l(mutex_);
  if (ref_count_ == 0) {
    counters_ = PerfCounters::Create(counter_names);
  }
  // We chose to increment it even if `counters_` ends up invalid,
  // so that we don't keep trying to create, and also since the dtor
  // will decrement regardless of `counters_`'s validity
  ++ref_count_;

  BM_CHECK(!counters_.IsValid() || counters_.names() == counter_names);
}

PerfCountersMeasurement::~PerfCountersMeasurement() {
  MutexLock l(mutex_);
  --ref_count_;
  if (ref_count_ == 0) {
    counters_ = PerfCounters::NoCounters();
  }
}

PerfCounters& PerfCounters::operator=(PerfCounters&& other) noexcept {
  if (this != &other) {
    CloseCounters();

    counter_ids_ = std::move(other.counter_ids_);
    counter_names_ = std::move(other.counter_names_);
  }
  return *this;
}
}  // namespace internal
}  // namespace benchmark