dotnet/BenchmarkDotNet

Generic benchmarks in one report

migoldfinger opened this issue · 10 comments

Hi,
I want to generate a report that show the performance difference between the different data types I generated a generic benchmarkclass for this,, it ends everytime with the benchmarks written in several files and not all benchmarks in one file and one table, how can I do that.

Hi @migoldfinger

From https://benchmarkdotnet.org/articles/samples/IntroJoin.html?q=join#sample-introjoin:

use the --join option (or BenchmarkSwitcher.RunAllJoined)

Remember that in order to get command line arguments respected you need to pass args to BenchmarkSwitcher/Runner

@adamsitnik No NOT closed

If I use --join I got this error.
// * You use JoinSummary options, but provided configurations cannot be joined. Only one Orderer per benchmark cases is allowed.

The reason is as soon as I add a second GenericTypeArgumentsAttribute to the class this error appears. So --join is no solution at all and your provides solution is invalid.

Just do be clear it seams that --join is not compatible with GenericTypeArgumentsAttribute and ConfigAttribute in combination.
Looks like there is a bug in the system with that combination.
If I remove the ConfigAttribute it works as expected.

@migoldfinger Can you please share your code for a repro?

@timcassell
Easy!
Works with every Generic Benchmark every time.
Just clone for example https://github.com/mjebrahimi/DotNet-Collections-Benchmark.git

Change line 19 in Program.cs to (just adding the args)
var benchmarkInfo = BenchmarkAutoRunner.SwitcherRun(typeof(Program).Assembly, args).GetBenchmarkInfo().ToArray();

Try to run with --join --filter *
Does not matter if there are many or just one generic benchmark class the error pops up every time.

@migoldfinger So, the error message tells you the issue.

Only one Orderer per benchmark cases is allowed.

I took a look at your code and saw this.

[CategoriesColumn]
[MemoryDiagnoser(displayGenColumns: false)]
[Config(typeof(CustomConfig))]
#if RELEASE
[ShortRunJob]
//[SimpleJob(BenchmarkDotNet.Engines.RunStrategy.Throughput)]
#else
[MarkdownExporterAttribute.GitHub]
#endif
public class BenchmarkBase

I assume all your benchmark classes are inheriting from this base class. The issue is the [Config(typeof(CustomConfig))] becomes a separate config on each benchmark class (and on each generic instantiation of your generic benchmark class) which all get merged together. It worked when I removed that attribute from the class. I tried applying the attribute to the assembly instead, but I still hit the error. You could fix it by instead passing it in to the benchmark switcher.

I got it to work by changing your CustomConfig constructor to include the same options as ManualConfig.CreateMinimumViable:

public CustomConfig()
{
    AddColumnProvider(DefaultColumnProviders.Instance);
    AddLogger(ConsoleLogger.Default);
    Orderer = new CustomOerderer();
    SummaryStyle = DefaultConfig.Instance.SummaryStyle.WithMaxParameterColumnWidth(50);
}

Then removed it from the class attribute and instead passed it directly to the benchmark switcher:

var benchmarkInfo = BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args, new CustomConfig()).GetBenchmarkInfo().ToArray();

@AndreyAkinshin Is that config merge working as expected? Or is there a bug in the validation? I would expect applying a single config to the assembly would succeed at least.

@timcassell, thanks for the investigation! This was a "by design" behavior, but now I feel that this is completely wrong.

Background

This problem emerges due to the unfortunate Config design. Right now, it mixes both "how to execute benchmarks" and "how to present results." The "how to execute" part is often scoped to a single benchmark or benchmark class; it's not a problem when different benchmark subsets have different "how to execute" descriptions. The "how to present" part is supposed to be applied for the whole summary table, but now can be configured individually for each benchmark class. When we join benchmark classes with different "how to present" descriptions, we cannot merge unambiguously. Here is a minimal repro for the current problem:

public class Config : ManualConfig
{
    public Config() => Orderer = new DefaultOrderer();
}

[Config(typeof(Config))]
public class BenchBase;

[DryJob]
public class Bench1 : BenchBase
{
    [Benchmark]
    public void Foo() { }
}

[DryJob]
public class Bench2 : BenchBase
{
    [Benchmark]
    public void Foo() { }
}

public class Program
{
    public static void Main()
    {
        BenchmarkSwitcher.FromTypes([typeof(Bench1), typeof(Bench2)]).Run(["--join", "--filter", "*"]);
    }
}

Two benchmark classes Bench1 and Bench2 get their own instances of Config with different instances of Orderer (can't be merged via Distinct). We cannot decide which Orderer to use in the summary table and, therefore, we fail with the following message:

// Validating benchmarks:
//    * You use JoinSummary options, but provided configurations cannot be joined. Only one Orderer per benchmark cases is allowed.

Long-term approach

I want to completely split the configuration of execution and presentation on the API level, so that it would be impossible to create such controversal situations. It's a part of my long-lasting refactoring I'm working on in background. Unfortunately, it turned out to be more challenging than I though in the beginning: we have two many "mixtures" of logic and presentation. It seems that it would be impossible to merge everything at once, so my current strategy is to try to backport small parts of my grand refactiring one-by-one.

Short-term solution

I feel it will be absolute OK to just disable this validation (remove ConfigCompatibilityValidator) and patch the logic of how we choose Order and SummaryStyle in the joined case:

diff --git a/src/BenchmarkDotNet/Reports/Summary.cs b/src/BenchmarkDotNet/Reports/Summary.cs
--- a/src/BenchmarkDotNet/Reports/Summary.cs    (revision a58872b82739b14935217adfa25db209d4419f4f)
+++ b/src/BenchmarkDotNet/Reports/Summary.cs    (date 1724696441775)
@@ -152,7 +152,7 @@
                    .Where(config => config.Orderer != DefaultOrderer.Instance)
                    .Select(config => config.Orderer)
                    .Distinct()
-                   .SingleOrDefault()
+                   .FirstOrDefault()
                ?? DefaultOrderer.Instance;
 
         private static SummaryStyle GetConfiguredSummaryStyleOrDefaultOne(ImmutableArray<BenchmarkCase> benchmarkCases)
@@ -165,7 +165,7 @@
 #nullable enable
                    .Select(benchmark => benchmark.Config.SummaryStyle)
                    .Distinct()
-                   .SingleOrDefault()
+                   .FirstOrDefault()
                ?? SummaryStyle.Default;
     }
 }

Effectively, instead of failing with validation message, we will correctly run all the benchmarks and present the summary table using the first provided Orderer and SummaryStyle.

@timcassell what do you think?

Sounds good to me. Perhaps the validation could be changed to a warning instead of removing altogether, in case someone does accidentally use 2 different orderers with different logic.

@timcassell I think we can go without warnings. Reasons:

  1. If the user is satisfied with the summary table style/ordering, we don't have any problems. If the user is not satisfied with the summary table style/ordering (because they provided inconsistent configs for various benchmarks), they will notice the problem without warnings. I don't see any cases, in which such a warning will be actually useful.
  2. Long-term, we don't need such a warning because we aim to achieve such an API that would not allow controversial setups. Thus, if we introduce such a warning, it will still be eliminated eventually, but meanwhile, we are supposed to maintain it. I'm OK with temporary solutions while I see actual benefits, but I don't like to introduce them "just in case."