Skip to content

Commit

Permalink
[Tooling] Rewrite generate_difference_report(). (#678)
Browse files Browse the repository at this point in the history
My knowledge of python is not great, so this is kinda horrible.

Two things:
1. If there were repetitions, for the RHS (i.e. the new value) we were always using the first repetition,
    which naturally results in incorrect change reports for the second and following repetitions.
    And what is even worse, that completely broke U test. :(
2. A better support for different repetition count for U test was missing.
    It's important if we are to be able to report 'iteration as repetition',
    since it is rather likely that the iteration count will mismatch.

Now, the rough idea on how this is implemented now. I think this is the right solution.
1. Get all benchmark names (in order) from the lhs benchmark.
2. While preserving the order, keep the unique names
3. Get all benchmark names (in order) from the rhs benchmark.
4. While preserving the order, keep the unique names
5. Intersect `2.` and `4.`, get the list of unique benchmark names that exist on both sides.
6. Now, we want to group (partition) all the benchmarks with the same name.
   ```
   BM_FOO:
       [lhs]: BM_FOO/repetition0 BM_FOO/repetition1
       [rhs]: BM_FOO/repetition0 BM_FOO/repetition1 BM_FOO/repetition2
   ...
   ```
   We also drop mismatches in `time_unit` here.
   _(whose bright idea was it to store arbitrarily scaled timers in json **?!** )_
7. Iterate for each partition
7.1. Conditionally, diff the overlapping repetitions (the count of repetitions may be different.)
7.2. Conditionally, do the U test:
7.2.1. Get **all** the values of `"real_time"` field from the lhs benchmark
7.2.2. Get **all** the values of `"cpu_time"` field from the lhs benchmark
7.2.3. Get **all** the values of `"real_time"` field from the rhs benchmark
7.2.4. Get **all** the values of `"cpu_time"` field from the rhs benchmark
          NOTE: the repetition count may be different, but we want *all* the values!
7.2.5. Do the rest of the u test stuff
7.2.6. Print u test
8. ???
9. **PROFIT**!

Fixes #677
  • Loading branch information
LebedevRI authored Sep 19, 2018
1 parent 439d6b1 commit aad33aa
Show file tree
Hide file tree
Showing 3 changed files with 256 additions and 111 deletions.
18 changes: 17 additions & 1 deletion tools/gbench/Inputs/test3_run0.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"name": "BM_Two",
"iterations": 1000,
"real_time": 8,
"cpu_time": 80,
"cpu_time": 86,
"time_unit": "ns"
},
{
Expand All @@ -37,13 +37,29 @@
"cpu_time": 80,
"time_unit": "ns"
},
{
"name": "short",
"run_type": "aggregate",
"iterations": 1000,
"real_time": 8,
"cpu_time": 77,
"time_unit": "ns"
},
{
"name": "medium",
"run_type": "iteration",
"iterations": 1000,
"real_time": 8,
"cpu_time": 80,
"time_unit": "ns"
},
{
"name": "medium",
"run_type": "iteration",
"iterations": 1000,
"real_time": 9,
"cpu_time": 82,
"time_unit": "ns"
}
]
}
22 changes: 19 additions & 3 deletions tools/gbench/Inputs/test3_run1.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,31 @@
"name": "BM_Two",
"iterations": 1000,
"real_time": 7,
"cpu_time": 70,
"cpu_time": 72,
"time_unit": "ns"
},
{
"name": "short",
"run_type": "aggregate",
"iterations": 1000,
"real_time": 8,
"cpu_time": 80,
"real_time": 7,
"cpu_time": 75,
"time_unit": "ns"
},
{
"name": "short",
"run_type": "aggregate",
"iterations": 762,
"real_time": 4.54,
"cpu_time": 66.6,
"time_unit": "ns"
},
{
"name": "short",
"run_type": "iteration",
"iterations": 1000,
"real_time": 800,
"cpu_time": 1,
"time_unit": "ns"
},
{
Expand Down
Loading

0 comments on commit aad33aa

Please sign in to comment.