Skip to content

Commit

Permalink
Drop failing attempts to actually verify the test results.
Browse files Browse the repository at this point in the history
To much noise.
  • Loading branch information
LebedevRI committed Feb 17, 2019
1 parent f397767 commit def9e0c
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 270 deletions.
2 changes: 2 additions & 0 deletions src/benchmark_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ class BenchmarkRunner {
// Adjust real/manual time stats since they were reported per thread.
i.results.real_time_used /= b.threads;
i.results.manual_time_used /= b.threads;
// If we were measuring whole-process CPU usage, adjust the CPU time too.
if (b.measure_process_cpu_time) i.results.cpu_time_used /= b.threads;

VLOG(2) << "Ran in " << i.results.cpu_time_used << "/"
<< i.results.real_time_used << "\n";
Expand Down
Loading

3 comments on commit def9e0c

@bryan-lunt
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't entirely understand the changes to benchmark_runner.cc . Is this taking our results of the total CPU time used and now not displaying that?

@LebedevRI
Copy link
Collaborator Author

@LebedevRI LebedevRI commented on def9e0c Feb 17, 2019

Choose a reason for hiding this comment

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

if benchmark itself runs benchmark in threads, it will accumulate (sum) the timings
of each of the threads. But if we were measuring the total cpu time, then each thread
will already measure the time of all the threads, and when we sum that..
So we have to, in some way, only take that measurement from just one thread.
Dividing by the number of threads should be good enough.

To be noted, i suspect all the timings are broken (== unreliable, incorrect) anyway
when ->Threads() is used, i have filed #769

@bryan-lunt
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I understand. Thanks.

Please sign in to comment.