Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IterationSetup is not running before each benchmark invocation #730

Closed
stephentoub opened this issue Apr 25, 2018 · 6 comments · Fixed by #764
Closed

IterationSetup is not running before each benchmark invocation #730

stephentoub opened this issue Apr 25, 2018 · 6 comments · Fixed by #764
Assignees
Milestone

Comments

@stephentoub
Copy link
Member

stephentoub commented Apr 25, 2018

Repro using 0.10.14:

public class Test
{
    public static void Main() => BenchmarkRunner.Run<Test>();

    [IterationSetup]
    public void MySetup() => Console.WriteLine("MySetup");

    [Benchmark]
    public void MyBenchmark()
    {
        Console.WriteLine("MyBenchmark");
        Thread.Sleep(100);
    }
}

Example output from the main run:

MainTarget  1: 16 op, 1608514852.97 ns, 100.5322 ms/op
MySetup
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MainTarget  2: 16 op, 1605497716.92 ns, 100.3436 ms/op
MySetup
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MainTarget  3: 16 op, 1610182705.89 ns, 100.6364 ms/op
MySetup
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark
MyBenchmark

This leads to very incorrect benchmark results when MyBenchmark is doing something that ends up caching data that can affect subsequent invocations of MyBenchmark. The whole purpose of IterationSetup in my case is to ensure that cache is cleared before the benchmark is executed.

@AndreyAkinshin
Copy link
Member

AndreyAkinshin commented Apr 27, 2018

The problem: the default value of UnrollFactor is 16. It means that we call the target method 16 times in the loop body. We need this trick only for nanobenchmark (it prevents some microarch effects). Obviously, it doesn't make sense to use IterationSetup (which is not designed for nanobenchmarks) and UnrollFactor (which is disigned only for nanobenchmarks) together.
Thus, we have to:

  • Use UnrollFactor=1 if IterationSetup or IterationCleanup is defined
  • Write a validator which prevents running benchmarks with IterationSetup/IterationCleanup and UnrollFactor>1

@adamsitnik
Copy link
Member

I have an idea for solving this problem:

Add new attribute: [BenchmarkSetup] which would be internally translated to [IterationSetup] with the right settings (Monitoring strategy etc)

@stephentoub would [BenchmarkSetup] name be less confusing?

@stephentoub
Copy link
Member Author

In what scenario is the current behavior useful? It just seems buggy to me.

@AndreyAkinshin
Copy link
Member

@adamsitnik I don't think that we should introduce a new attribute. We just shouldn't use both IterationSetup/IterationCleanup and UnrollFactor>1 together, it's an invalid config.

@Rizzen
Copy link
Contributor

Rizzen commented May 6, 2018

@AndreyAkinshin I guess that is not enough just change UnrollFactor due to calculating invokeCount during pilot stage.

@adamsitnik
Copy link
Member

I will solve this issue soon, it is causing too much confusion.

When IterationSetup is used, the UnrollFactor=InvocationCount=1

adamsitnik added a commit that referenced this issue May 27, 2018
…re not, run benchmark once per iteration to avoid user confusion, fixes #730
alinasmirnova pushed a commit to alinasmirnova/BenchmarkDotNet that referenced this issue Sep 22, 2018
…re not, run benchmark once per iteration to avoid user confusion, fixes dotnet#730
alinasmirnova pushed a commit to alinasmirnova/BenchmarkDotNet that referenced this issue Sep 22, 2018
SnakyBeaky added a commit to SnakyBeaky/BenchmarkDotNet that referenced this issue Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment