Skip to content

Update wording for AggressiveOptimizations #5355

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Update wording for AggressiveOptimizations #5355

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion xml/System.Runtime.CompilerServices/MethodImplOptions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@
</ReturnValue>
<MemberValue>512</MemberValue>
<Docs>
<summary>The method contains a hot path and should be optimized.</summary>
<summary>The method should skip tiered-compilation and immediately reach tier0.
Copy link
Contributor

@SingleAccretion SingleAccretion Feb 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably, Tier1. However, I do not think this terminology is used anywhere in the public-facing documentation.

I would suggest just dropping the " and immediately reach tier0" part.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pasting that tag from the C# server if you want to inteegrate some of it in the remarks section 😄

The AggressiveOptimization option is meant for very specific cases and should only ever be used when sure that the method being annotated actually benefits from that (possibly running some benchmarks to confirm that). Despite what the name might suggest, this option will not "make your code faster", on the contrary it might very well make it slower in many situations. AggressiveOptimization means that the method will skip the tiered compilation and go directly to tier1. This can be beneficial for methods that are hot paths or long running, but only executed once, so that the tiered JIT wouldn't have time to kick in (for instance, for a complex app initialization method that is run at startup). But skipping the tier0 -> tier1 transition also means that a number of additional optimizations cannot be done by the JIT (eg. removing checks for the static constructor, or inlining static readonly fields). Because of this, in many cases applying this option might actually make your code slower. And regardless, the JIT will often skip tier0 anyway in many cases already (eg. in methods with a backwards branch), so it might very well be unnecessary anyway. Long story short: if you're not 100% sure, just don't use this option in your code.

This can be beneficial for certain hot-path code which does not rely on any static state or any optimisations applied at re-jit time.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which optimizations, which paths are "certain"? As a someone who is looking to optimize my code and not sure whether I should use this attribute or not, I am afraid this is too little detail, at the same time, I do not know how to better word this...

I think the general guidance of: "this method is known to be hot, does not rely on state that is known only after is has been executed, and is on a startup path" is OK'ish?

At the same time, no Tier0 means no profile data and no PGO, so in a sense, all methods should benefit, except if they are hand-optimized with gotos and such.

This attribute will not result in faster or better optimised code and is designed for use in very specific benchmark-lead scenarios.</summary>
</Docs>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This describes what this attribute does in the current coreclr runtime. I expect that the details will change a lot over time. We typically refrain from documenting implementation details like this since it makes the documentation constantly outdated.

The method contains a hot path and should be optimized

All methods are optimized. I would change the description to "The method contains a very critical hot path and should be optimized with highest optimization level possible.".

But I think we should avoid describing the current implementation details of coreclr runtime.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with Jan.

I also wouldn't connect it to benchmarking.

</Member>
<Member MemberName="ForwardRef">
Expand Down