-
Notifications
You must be signed in to change notification settings - Fork 2.7k
When QuickJit is enabled, disable it for methods that contain loops by default #24252
Conversation
…y default Fixes https://github.com/dotnet/coreclr/issues/19751 by default when QuickJit is enabled - Added config variable TC_QuickJitForLoops. When disabled (the default), the JIT identifies loops and explicit tail calls and switches to tier 1 JIT. - This would prevent the possibility of spending too long in QuickJit code, but may decrease startup time a bit when QuickJit is enabled - Removed TC_StartupTier_OptimizeCode, as now that there is TC_QuickJit, I didn't see a good use for it - Removed references to "StartupTier" in config variables because we had previously decided not to call it that. - When QuickJit is disabled, avoid creating native code slots for methods in non-R2R'ed modules, as tiering would be disabled for those anyway
`TC_StartupTier_CallCountThreshold` | Number of times a method must be called in the startup tier after which it is promoted to the next tier. | `DWORD` | `UNSUPPORTED` | `30` | | ||
`TC_StartupTier_DelaySingleProcMultiplier` | Multiplier for TC_StartupTier_CallCountingDelayMs that is applied on a single-processor machine or when the process is affinitized to a single processor. | `DWORD` | `UNSUPPORTED` | `10` | | ||
`TC_StartupTier_OptimizeCode` | Use optimized codegen (normally used by the optimized tier) in the startup tier | `DWORD` | `INTERNAL` | `0` | | ||
`TC_QuickJitForLoops` | When quick JIT is enabled, quick JIT may also be used for methods that contain loops. | `DWORD` | `UNSUPPORTED` | `0` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be EXTERNAL
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(same for TC_QuickJit
?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think TC_QuickJit
should be "external" to indicate that it is supported. For TC_QuickJitForLoops
, "unsupported" is probably appropriate. It doesn't affect whether the options can be configured with any means.
Below are some startup time perf numbers. No significant difference to steady-state perf.
JitBench startup times (ms)
Diff from Tier,NoQJ:
PowerShell startup times (ms)
Diff from Tier,NoQJ:
ASP.NET startup times (server start + first request, ms)
Diff from Tier,NoQJ:
|
@dotnet-bot test Windows_NT x64 Formatting |
@dotnet-bot test Windows_NT x64 Release CoreFX Tests |
@dotnet/jit-contrib, would anyone like to take a look at the changes on the JIT side? |
From the data presented it appears that always using QuickJit produces even better startup times:
Is there some data that supports not using QuickJit for methods that (might) contain loops? |
It's usually better to enable QuickJit for methods that contain loops. Sometimes though the loop can run for a long time and it would not be optimized ever, or may be optimized late. This usually shows up in simple microbenchmarks, such as the test case in https://github.com/dotnet/coreclr/issues/19751. It can also show up when using BenchmarkDotNet if each invocation of the test takes long enough for BDN to run only a few invocations per iteration. It can happen in non-benchmark code where a method may only run once, like Main or the start method of a long-running thread with an infinite loop. Several workarounds were provided for this issue (AggressiveOptimization attribute, config switches). The issue is showing up frequently with people using 3.0 preview builds and the change is being made to limit risk of the known issue for which we don't have a good solution at the moment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some suggestions for names that I think might be clearer, but am happy to hear arguments to keep them as-is. I'd also like to see function header comments.
…y default (dotnet/coreclr#24252) When QuickJit is enabled, disable it for methods that contain loops by default Fixes https://github.com/dotnet/coreclr/issues/19751 by default when QuickJit is enabled - Added config variable TC_QuickJitForLoops. When disabled (the default), the JIT identifies loops and explicit tail calls and switches to tier 1 JIT. - This would prevent the possibility of spending too long in QuickJit code, but may decrease startup time a bit when QuickJit is enabled - Removed TC_StartupTier_OptimizeCode, as now that there is TC_QuickJit, I didn't see a good use for it - Removed references to "StartupTier" in config variables because we had previously decided not to call it that. - When QuickJit is disabled, avoid creating native code slots for methods in non-R2R'ed modules, as tiering would be disabled for those anyway - Marked TC_QuickJit config var as external Commit migrated from dotnet/coreclr@607c8db
Fixes https://github.com/dotnet/coreclr/issues/19751 by default when QuickJit is enabled