Issue #1024: Calculate baseline by the fastest benchmark

[Serg046]

trafficstars

Fixes #1024 The idea is inside Summary.cs

[adamsitnik]

I am not very familiar with this part of BDN and if possible I would prefer @AndreyAkinshin to review it. Thanks!

[Serg046]

I tried to fix all the notes we have here, please let me know if there is anything else to deal with @YegorStepanov @AndreyAkinshin @adamsitnik

[YegorStepanov]

Cons of the current implementation:

We cannot extend the enum, because attribute actually set a boolean flag in config. It make sense to drop AutomaticBaselineMode completely. (Attribute will set IConfig.IsAutomaticBaseline = true).

I like the current attribute API but it will need to rename the attribute, if we drop the enum.

I'm thinking of a more powerful solution:

Not for all users Mean is the most interesting column:

• In the real-time apps such as games, allocation is more painful than mean time.
• Andrey will like this: Mean is never interesting. Compare methods by percentage (like P90) or at least by Median.
• Indeed, all the math columns can be interesting here.

API:

[AutomaticBaseline(string columnName, Strategy strategy = Maximum)]

enum Strategy { Minimum, Maximum } // AutomaticBaselineStrategy or AutomaticStrategy or BaselineStrategy?

how to use:

[AutomaticBaseline("P90")]
[AutomaticBaseline(Column.P90))]

We can compare values from the string IColumn.GetValue(...) method (if IColumn.IsNumeric == true than we do TryParse and compare them by number, not by string). Some columns such as Runtime need special handling.

Questions:

Does it make sense to add support for assembly: to attribute?

[assembly: [AutomaticBaseline("Mean")]]

public Benchmarks
{
    // currently, it would be an error: "Do not use both AutomaticBaseline and [Benchmark(Baseline = true)]"
    [Benchmark(Baseline = true)] public void B1() { }
    [Benchmark] public void B1() => Thread.Sleep(100);
}

How can this be resolved:

1. make assembly: attribute overridable by [Benchmark(Baseline = true)] and [class: AutomaticBaseline("another-column")]
2. add parameter: [AutomaticBaseLine(..., bool overridable = false)]

If Serg046 does not have so much time, then I can do it within a few months.

[Serg046]

The idea looks very interesting to me. If mainterners like it too, I am also open to make it happen. The only thought I have is that it looks as a separate feature. I think this should be working for both automatic and explicit/manual baseline modes, while this PR is focused on a new auto one only. We can create a new issue and discuss a feature design there. However, if we want to do something here, I am still ready for that!

[YegorStepanov]

Are both solutions really needed? They look very similar:

[AutomaticBaseline(AutomaticBaselineMode.Fastest)]
[AutomaticBaseline("Mean")] // how to name it?

AutomaticBaselineMode.Fastest

As I wrote above, it is not clear what the Fastest means.

Why is it compared by the Mean and not the Median? Maybe BDN calculates Fastest benchmark with complex formulas based on benchmark iterations?

If we change it to the Median, users may think that is a bug, because by default the Median column is not shown, only the Mean column.

[Serg046]

Are both solutions really needed?

No, I meant it would be great to suport it for manual/explicit mode as well as for the new AutomaticBaseline attribute. Something like that:

public Benchmarks
{
    // Existing functionality with a new feature
    [Benchmark(Baseline = true, BaselineMetric = "Mean")] public void B1() { }
}

Why is it compared by the Mean and not the Median?

Just my choice during development. I am fine to change this default to something else, e.g. Median like you say.

If we change it to the Median, users may think that is a bug, because by default the Median column is not shown, only the Mean column.

Exactly

[AndreyAkinshin]

The Ratio column is not just the ratio of means (and it shouldn't be a ratio of other statistics). The current implementation can be found in BenchmarkDotNet.Mathematics.Statistics.Divide, but I'm not happy with it. I'm working on a new approach based on ratio functions (which is a part of a new summary table with a better statistical engine; the research notes can be found in my blog). tl;dr: avoid using specific metrics in the baseline API.