Move initialization of YieldProcessorNormalized to the finalizer thread #14058
Conversation
Fixes https://github.com/dotnet/coreclr/issues/13984 - Also moved relevant functions out of the Thread class as requested in the issue - For some reason, after moving the functions out of the Thread class, YieldProcessorNormalized was not getting inlined anymore. It seems to be important to have it be inlined such that the memory loads are hoisted out of outer loops. To remove the dependency on the compiler to do it (even with forceinline it's not possible to hoist sometimes, for instance InterlockedCompareExchnage loops), changed the signatures to do what is intended.
kouvel
force-pushed
the
MoveYpNormalized
branch
from
242c237 to
5cb8253
Compare
src/vm/threads.cpp
Outdated
|
|
||
| // Defaults are for when InitializeYieldProcessorNormalized has not yet been called or when no measurement is done, and are | ||
| // tuned for Skylake processors | ||
| int g_yieldsPerNormalizedYield = 1; // 9 for pre-Skylake |
There was a problem hiding this comment.
What does this "9 for pre-Skylake" comment mean?
There was a problem hiding this comment.
For pre-Skylake processors it would be about 9 yields per normalized yield
There was a problem hiding this comment.
Ok. It'd be helpful to expand the comment to highlight that this is an estimate for what the value will be, not that we're somewhere setting this to a hardcoded "9".
| @@ -744,6 +744,8 @@ DWORD WINAPI FinalizerThread::FinalizerThreadStart(void *args) | |||
| #endif | |||
| GetFinalizerThread()->SetBackground(TRUE); | |||
|
|
|||
| EnsureYieldProcessorNormalizedInitialized(); | |||
There was a problem hiding this comment.
Am I understanding the change correctly that until the finalizer thread gets to this point, spinning will be done using a default value, and then once this runs, that value will be updated appropriately based on the system? I think that makes sense, just wanted to confirm.
|
I just realized that the scheme doesn't work for the back-off version, I'll change it around a bit |
|
Going to revoke your approval as a result |
|
Thanks |
I think it's ok to do that for this case, the other QCALL functions are organized in this way I think to make sure all of the time-consuming work happens between BEGIN_QCALL and END_QCALL so I just followed the same pattern. |
|
@dotnet-bot test Windows_NT x64 Release Priority 1 Build and Test |
Fixes https://github.com/dotnet/coreclr/issues/13984