Add grouping by type #2163
Add grouping by type #2163
Conversation
| BenchmarkLogicalGroupRule.ByCategory, | ||
| BenchmarkLogicalGroupRule.ByType, |
There was a problem hiding this comment.
ByType should be first by default, I think.
There was a problem hiding this comment.
I don't know :)
I reordered it and the output didn't change for the tests.
I tried to sort it twice - all tests are passed:
[GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByType)]
[GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByMethod)]
class A { }I did it for
1) for each rule + ByType
2) for each rule + ByCategory
3) for each rule + ByMethod
but the output is same
There was a problem hiding this comment.
Everything is right, nothing has changed 🤔
There was a problem hiding this comment.
@YegorStepanov What types are you testing? Do the benchmarks have enough options that the table should actually be different for each group rule-set?
There was a problem hiding this comment.
I would expect TwoBaselinesWithJobs111 and TwoBaselinesWithJobs222 to be the same. And TwoBaselinesWithJobs333 should order by Type first, then Method. For reasons I explained in my other comment.
There was a problem hiding this comment.
Tbh, I'm starting to think ByType shouldn't even be override-able. Each Type is meant to be benchmarking a different function, so why would they be grouped any other way? [Edit] The only reason I can think of it needing to be changed is to put ByRuntime in front of it or vice-versa, but that doesn't currently exist. ByType should never be ordered after any of the existing logical group rules.
There was a problem hiding this comment.
The output for TwoBaselinesWithJobs111 and TwoBaselinesWithJobs222 is the same (they are slightly differs in the LogicalGroup column).
ByCategory:
TwoBaselinesWithJobs111
Type | Method | Job | Categories | Param | Mean | Error | StdDev | Ratio | RatioSD | LogicalGroup | Baseline |
------- |------- |----------- |----------- |------ |---------:|--------:|--------:|------:|--------:|--------------------------------- |--------- |
Bench1 | Foo | Job1 | A | 10 | 102.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-A-Bench1-[Param=10]-Job1 | Yes | ^
Bench1 | Bar | Job1 | A | 10 | 202.0 ns | 6.09 ns | 1.58 ns | 1.98 | 0.02 | A-A-Bench1-[Param=10]-Job1 | No |
| | | | | | | | | | | |
Bench1 | Foo | Job2 | A | 10 | 302.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-A-Bench1-[Param=10]-Job2 | Yes |
Bench1 | Bar | Job2 | A | 10 | 402.0 ns | 6.09 ns | 1.58 ns | 1.33 | 0.00 | A-A-Bench1-[Param=10]-Job2 | No |
| | | | | | | | | | | |
Bench1 | Foo | Job1 | A | 20 | 502.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-A-Bench1-[Param=20]-Job1 | Yes | ^
Bench1 | Bar | Job1 | A | 20 | 602.0 ns | 6.09 ns | 1.58 ns | 1.20 | 0.00 | A-A-Bench1-[Param=20]-Job1 | No |
| | | | | | | | | | | |
Bench1 | Foo | Job2 | A | 20 | 702.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-A-Bench1-[Param=20]-Job2 | Yes |
Bench1 | Bar | Job2 | A | 20 | 802.0 ns | 6.09 ns | 1.58 ns | 1.14 | 0.00 | A-A-Bench1-[Param=20]-Job2 | No |
| | | | | | | | | | | |
Bench2 | Foo | DefaultJob | B | 10 | 102.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | B-B-Bench2-[Param=10]-DefaultJob | Yes | ^
Bench2 | Bar | DefaultJob | B | 10 | 202.0 ns | 6.09 ns | 1.58 ns | 1.98 | 0.02 | B-B-Bench2-[Param=10]-DefaultJob | No |
| | | | | | | | | | | |
Bench2 | Foo | DefaultJob | B | 20 | 302.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | B-B-Bench2-[Param=20]-DefaultJob | Yes | ^
Bench2 | Bar | DefaultJob | B | 20 | 402.0 ns | 6.09 ns | 1.58 ns | 1.33 | 0.00 | B-B-Bench2-[Param=20]-DefaultJob | No |
TwoBaselinesWithJobs222
Type | Method | Job | Categories | Param | Mean | Error | StdDev | Ratio | RatioSD | LogicalGroup | Baseline |
------- |------- |----------- |----------- |------ |---------:|--------:|--------:|------:|--------:|------------------------------- |--------- |
Bench1 | Foo | Job1 | A | 10 | 102.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-Bench1-[Param=10]-Job1 | Yes | ^
Bench1 | Bar | Job1 | A | 10 | 202.0 ns | 6.09 ns | 1.58 ns | 1.98 | 0.02 | A-Bench1-[Param=10]-Job1 | No |
| | | | | | | | | | | |
Bench1 | Foo | Job2 | A | 10 | 302.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-Bench1-[Param=10]-Job2 | Yes |
Bench1 | Bar | Job2 | A | 10 | 402.0 ns | 6.09 ns | 1.58 ns | 1.33 | 0.00 | A-Bench1-[Param=10]-Job2 | No |
| | | | | | | | | | | |
Bench1 | Foo | Job1 | A | 20 | 502.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-Bench1-[Param=20]-Job1 | Yes | ^
Bench1 | Bar | Job1 | A | 20 | 602.0 ns | 6.09 ns | 1.58 ns | 1.20 | 0.00 | A-Bench1-[Param=20]-Job1 | No |
| | | | | | | | | | | |
Bench1 | Foo | Job2 | A | 20 | 702.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-Bench1-[Param=20]-Job2 | Yes |
Bench1 | Bar | Job2 | A | 20 | 802.0 ns | 6.09 ns | 1.58 ns | 1.14 | 0.00 | A-Bench1-[Param=20]-Job2 | No |
| | | | | | | | | | | |
Bench2 | Foo | DefaultJob | B | 10 | 102.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | B-Bench2-[Param=10]-DefaultJob | Yes | ^
Bench2 | Bar | DefaultJob | B | 10 | 202.0 ns | 6.09 ns | 1.58 ns | 1.98 | 0.02 | B-Bench2-[Param=10]-DefaultJob | No |
| | | | | | | | | | | |
Bench2 | Foo | DefaultJob | B | 20 | 302.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | B-Bench2-[Param=20]-DefaultJob | Yes | ^
Bench2 | Bar | DefaultJob | B | 20 | 402.0 ns | 6.09 ns | 1.58 ns | 1.33 | 0.00 | B-Bench2-[Param=20]-DefaultJob | No |
TwoBaselinesWithJobs333
Type | Method | Job | Categories | Param | Mean | Error | StdDev | Ratio | RatioSD | LogicalGroup | Baseline |
------- |------- |----------- |----------- |------ |---------:|--------:|--------:|------:|--------:|----------------------------------- |--------- |
Bench1 | Foo | Job1 | A | 10 | 102.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-Foo-Bench1-[Param=10]-Job1 | Yes | ^
| | | | | | | | | | | |
Bench1 | Foo | Job2 | A | 10 | 302.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-Foo-Bench1-[Param=10]-Job2 | Yes |
| | | | | | | | | | | |
Bench1 | Foo | Job1 | A | 20 | 502.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-Foo-Bench1-[Param=20]-Job1 | Yes | ^
| | | | | | | | | | | |
Bench1 | Foo | Job2 | A | 20 | 702.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | A-Foo-Bench1-[Param=20]-Job2 | Yes |
| | | | | | | | | | | |
Bench1 | Bar | Job1 | A | 10 | 202.0 ns | 6.09 ns | 1.58 ns | ? | ? | A-Bar-Bench1-[Param=10]-Job1 | No | ^
| | | | | | | | | | | |
Bench1 | Bar | Job2 | A | 10 | 402.0 ns | 6.09 ns | 1.58 ns | ? | ? | A-Bar-Bench1-[Param=10]-Job2 | No |
| | | | | | | | | | | |
Bench1 | Bar | Job1 | A | 20 | 602.0 ns | 6.09 ns | 1.58 ns | ? | ? | A-Bar-Bench1-[Param=20]-Job1 | No | ^
| | | | | | | | | | | |
Bench1 | Bar | Job2 | A | 20 | 802.0 ns | 6.09 ns | 1.58 ns | ? | ? | A-Bar-Bench1-[Param=20]-Job2 | No |
| | | | | | | | | | | |
Bench2 | Foo | DefaultJob | B | 10 | 102.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | B-Foo-Bench2-[Param=10]-DefaultJob | Yes | ^
| | | | | | | | | | | |
Bench2 | Foo | DefaultJob | B | 20 | 302.0 ns | 6.09 ns | 1.58 ns | 1.00 | 0.00 | B-Foo-Bench2-[Param=20]-DefaultJob | Yes | ^
| | | | | | | | | | | |
Bench2 | Bar | DefaultJob | B | 10 | 202.0 ns | 6.09 ns | 1.58 ns | ? | ? | B-Bar-Bench2-[Param=10]-DefaultJob | No | ^
| | | | | | | | | | | |
Bench2 | Bar | DefaultJob | B | 20 | 402.0 ns | 6.09 ns | 1.58 ns | ? | ? | B-Bar-Bench2-[Param=20]-DefaultJob | No | ^
There was a problem hiding this comment.
I pushed it to another branch.
You can see it (although it's not easy):
YegorStepanov@8629a8b
There was a problem hiding this comment.
Do we need a ByRuntime grouping?
ByJob has EnvironmentCharacteristic, which has RuntimeCharacteristic.
It seems that ByJob should do your desired grouping.
Old, wrong answer
If [Edit] Oh wait, I think I misunderstood the question. Because the In that case, I think With that said, I think that table should look like this: |
|
A follow-up to this may want to add a |
|
I don't like that grouping How the grouping works:
|
|
I'm converting it to draft until I figure out why nothing changes due to swapping [Edit] |
YegorStepanov
force-pushed
the
byType-logical-rule
branch
from
337a489 to
5eb47d4
Compare
|
I made the most conservative mode. I don't like that there were more groups than I expected (look at the last commit) assign to @AndreyAkinshin |
|
@YegorStepanov I tried these changes and, while it looks better, there's still some weirdness with the table: It seems to be grouping the parameter ahead of the type, and the ratio column is not segregated for each type (it's shared for the entire parameter, regardless of type). public abstract class AsyncBenchmark
{
[Params(true, false)]
public bool Pending;
}
public partial class AsyncAwait : AsyncBenchmark
{
}
public partial class ContinueWith : AsyncBenchmark
{
} |
Fixes #1164
Fixes #2022
Look at "Update existing tests" and "Update new tests" to see what has changed.
Fixes:
#1164 (comment)
Master:
PR:
#2022 (comment)
Master:
PR
#1864 (comment)
Master
PR
#1864 (comment)
Master
PR
Tests
Yes, that's a lot of tests, but now it's easy to change the ordering logic as there are a lot of cases covered.
For example, this change produces a better output, but it breaks output for
OneBaselinein some test (it starts display baseline numbers instead a question mark):Question
What is the correct table for this?
Master/PR output is: