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

Ensure WorkloadActionUnroll and similar are optimized if possible #1934

Closed
AndyAyersMS opened this issue Mar 8, 2022 · 2 comments · Fixed by #1935 or #2335
Closed

Ensure WorkloadActionUnroll and similar are optimized if possible #1934

AndyAyersMS opened this issue Mar 8, 2022 · 2 comments · Fixed by #1935 or #2335
Milestone

Comments

@AndyAyersMS
Copy link
Member

If {Workload|Overhead}Action{No}Unroll are not optimized, and the benchmark method allocates a lot, the Tier0 versions of these methods may retain much more heap data than the optimized versions. This can completely change benchmark results.

Current behavior for CLR is that these methods are always eagerly optimized, because they all contain loops and we either do not have tiered compilation (full framework, pre-3.0 core) or have COMPlus_TC_QuickJitForLoops=1 (core 3.0 - core 6.0).

For .NET 7 we plan to change the default to COMPlus_TC_QuickJitForLoops=0 and this leads to these methods not being optimized and significant behavior changes in roughly 50 or so benchmarks on github.com/dotnet/performance.

To preserve current behavior, these methods should always be optimized when running against .NET Core versions that can change tiering strategy. This can be done by adding [MethodImpl(MethodImplOptions.AggressiveOptimziation)] which is available in .Net Core 3.0 and up.

Alternatively one could consider restructuring things so that the benchmark delegate is always called from a method that doesn't have loops. This seems more disruptive.

See dotnet/performance#2214 (comment) and following comments for context.

cc @adamsitnik

@AndyAyersMS
Copy link
Member Author

Also when Tiering or OSR is enabled these methods may shift performance over time. Seems like it is desirable for these methods not to change the code they're running.

@adamsitnik
Copy link
Member

@AndyAyersMS thank you for providing full explanation. I currently can't see any drawbacks of adding [MethodImpl(MethodImplOptions.AggressiveOptimziation)] to the mentioned methods.

@AndreyAkinshin what is your opinion on that?

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