Skip to content

Add opt-in cross-thread statistics for multithreaded runs - #2293

Open
Frank20021 wants to merge 1 commit into
google:mainfrom
Frank20021:report-thread-statistics
Open

Add opt-in cross-thread statistics for multithreaded runs#2293
Frank20021 wants to merge 1 commit into
google:mainfrom
Frank20021:report-thread-statistics

Conversation

@Frank20021

Copy link
Copy Markdown

Summary

  • Implements the design posted on [FR] Report cross-thread statistics for multithreaded runs #1902: opt-in mean/median/stddev/cv across threads of one multithreaded run.
  • Default summed (then Finish) counter row is unchanged. Extra RT_Aggregate rows use a thread_ prefix so they do not collide with repetition _mean / _median / _stddev / _cv.
  • Enable with ->ReportThreadStatistics() or --benchmark_report_thread_statistics=true. Per-thread samples are captured in RunInThread before Increment; ComputeStats still only sees iteration runs.

Test plan

  • thread_statistics_test (unequal {1,2,3} counters, kAvgThreads, feature off, threads:1)
  • user_counters_threads_test
  • repetitions_test --benchmark_repetitions=3

Fixes #1902

AI usage: I used Cursor as a coding assistant to locate the runner/reporter paths and draft this change. I reviewed the capture/Finish/ComputeStats wiring and the tests myself.

Keep the existing summed counter row unchanged, and when requested
report mean/median/stddev/cv across threads so fairness is visible.
@google-cla

google-cla Bot commented Sep 3, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Comment on lines +184 to +185
bool report_thread_statistics_specified_;
bool report_thread_statistics_;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need both? if the default is false does it matter if it's specified or not?

Comment thread src/benchmark_register.cc
}

Benchmark* Benchmark::ReportThreadStatistics(bool value) {
report_thread_statistics_specified_ = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does this interact with other per benchmark settings? do we need to make sure that the user hasn't set incompatible flags?

Comment thread src/benchmark_runner.cc

const IterationCount thread_iters = thread_results.front().iterations;

for (const auto& tr : thread_results) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like code we have elsewhere. is there any opportunity to share the code instead of duplicating it?

Comment thread src/benchmark_runner.h
bool warmup_done;
const int repeats;
const bool has_explicit_iteration_count;
const bool report_thread_statistics;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this to be a member?

@@ -0,0 +1,230 @@

#undef NDEBUG

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FR] Report cross-thread statistics for multithreaded runs

2 participants