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

don't require the JitDiagnosers (TailCall & Inlining) to run benchmarks once again just to gather JIT info, the overhead is very small #1375

Merged
merged 1 commit into from
Mar 2, 2020

Conversation

adamsitnik
Copy link
Member

While working on dotnet/runtime#2191 I got surprised by how long it takes to get the benchmark results.

It turned out that so far we were always running the benchmarks one more time if Inlining or TailCall diagnosers were enabled. We simply don't need to do that, the overhead of these two diagnosers is very, very small (we just sign up for 3 events).

git clone https://github.com/dotnet/BenchmarkDotNet/
cd BenchmarkDotNet
cd samples
cd BenchmarkDotNet.Samples.FSharp
dotnet run -c Release -f net461 --filter TailCallDetector

Before:

Method facRank Mean Error StdDev
test 7 9.460 ns 0.2158 ns 0.2310 ns
test1 7 4.758 ns 0.0824 ns 0.0731 ns
test2 7 51.820 ns 0.9814 ns 0.9180 ns
Run time: 00:02:29 (149.11 sec), executed benchmarks: 3

Global total time: 00:02:31 (151.73 sec), executed benchmarks: 3

After:

Method facRank Mean Error StdDev
test 7 9.383 ns 0.0837 ns 0.0783 ns
test1 7 4.688 ns 0.0655 ns 0.0613 ns
test2 7 49.748 ns 0.2633 ns 0.2199 ns
Run time: 00:01:23 (83.31 sec), executed benchmarks: 3

Global total time: 00:01:26 (86.03 sec), executed benchmarks: 3

…ks once again just to gather JIT info, the overhead is very small
@0xGeorgii
Copy link
Contributor

@adamsitnik make sense, I believe 👍

@adamsitnik
Copy link
Member Author

@GeorgePlotnikov thanks for a very fast response!

@AndreyAkinshin I need your approval to be able to merge

@adamsitnik
Copy link
Member Author

@AndreyAkinshin ping

@AndreyAkinshin AndreyAkinshin added this to the v0.12.1 milestone Mar 2, 2020
@AndreyAkinshin AndreyAkinshin merged commit b788bcb into master Mar 2, 2020
@AndreyAkinshin AndreyAkinshin deleted the noExtraRunForJitDiagnosers branch March 2, 2020 10:56
@AndreyAkinshin
Copy link
Member

@adamsitnik thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants