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.

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

Jump to bottom

Convert SpinWait to QCall #92675

Merged
merged 2 commits into from
Sep 27, 2023
Merged

Convert SpinWait to QCall #92675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0d5281
Convert SpinWait to QCall
jkotas Sep 26, 2023
ed595a3
Add SuppressGCTransition
jkotas Sep 26, 2023
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
28 changes: 24 additions & 4 deletions src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,35 @@ private void StartCallback()
[MethodImpl(MethodImplOptions.InternalCall)]
private static extern void SleepInternal(int millisecondsTimeout);

// Max iterations to be done in SpinWait without switching GC modes.
private const int SpinWaitCoopThreshold = 1024;

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ThreadNative_SpinWait")]
[SuppressGCTransition]
private static partial void SpinWaitInternal(int iterations);

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ThreadNative_SpinWait")]
private static partial void LongSpinWaitInternal(int iterations);

[MethodImpl(MethodImplOptions.NoInlining)] // Slow path method. Make sure that the caller frame does not pay for PInvoke overhead.
private static void LongSpinWait(int iterations) => LongSpinWaitInternal(iterations);

/// <summary>
/// Wait for a length of time proportional to 'iterations'. Each iteration is should
/// only take a few machine instructions. Calling this API is preferable to coding
/// a explicit busy loop because the hardware can be informed that it is busy waiting.
/// </summary>
[MethodImpl(MethodImplOptions.InternalCall)]
private static extern void SpinWaitInternal(int iterations);

public static void SpinWait(int iterations) => SpinWaitInternal(iterations);
public static void SpinWait(int iterations)
{
if (iterations < SpinWaitCoopThreshold)
{
SpinWaitInternal(iterations);
}
else
{
LongSpinWait(iterations);
}
}

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ThreadNative_YieldThread")]
private static partial Interop.BOOL YieldInternal();
Expand Down
23 changes: 1 addition & 22 deletions src/coreclr/vm/comsynchronizable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,7 @@ FCIMPL0(INT32, ThreadNative::GetOptimalMaxSpinWaitsPerSpinIteration)
}
FCIMPLEND

FCIMPL1(void, ThreadNative::SpinWait, int iterations)
extern "C" void QCALLTYPE ThreadNative_SpinWait(INT32 iterations)
{
FCALL_CONTRACT;

Expand All @@ -1044,29 +1044,8 @@ FCIMPL1(void, ThreadNative::SpinWait, int iterations)
return;
}

//
// If we're not going to spin for long, it's ok to remain in cooperative mode.
// The threshold is determined by the cost of entering preemptive mode; if we're
// spinning for less than that number of cycles, then switching to preemptive
// mode won't help a GC start any faster.
//
if (iterations <= 100000)
{
YieldProcessorNormalized(iterations);
Copy link
Member Author

Choose a reason for hiding this comment

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

This path was missing a GC probe that makes it unfriendly to GC suspension. It has proper GC probe now thanks to SuppressGCTransition.

Copy link
Member

@EgorBo EgorBo Sep 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Do I understand it correctly that a non-interruptable loop with potentially 1023 iterations of pauses is not an issue for gc starving?

Copy link
Member Author

@jkotas jkotas Sep 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Right, our docs say that starving the GC suspension for 1 microsecond is ok: https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.suppressgctransitionattribute

Copy link
Member Author

@jkotas jkotas Sep 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

(1023 iterations of pause may be more than that, but it does not really matter. It just needs to be in the right ballpark.)

return;
}

//
// Too many iterations; better switch to preemptive mode to avoid stalling a GC.
//
HELPER_METHOD_FRAME_BEGIN_NOPOLL();
GCX_PREEMP();

YieldProcessorNormalized(iterations);

HELPER_METHOD_FRAME_END();
}
FCIMPLEND

extern "C" BOOL QCALLTYPE ThreadNative_YieldThread()
{
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/comsynchronizable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ extern "C" BOOL QCALLTYPE ThreadNative_YieldThread();
extern "C" UINT64 QCALLTYPE ThreadNative_GetCurrentOSThreadId();
extern "C" void QCALLTYPE ThreadNative_Abort(QCall::ThreadHandle thread);
extern "C" void QCALLTYPE ThreadNative_ResetAbort();
extern "C" void QCALLTYPE ThreadNative_SpinWait(INT32 iterations);

#endif // _COMSYNCHRONIZABLE_H

1 change: 0 additions & 1 deletion src/coreclr/vm/ecalllist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,6 @@ FCFuncStart(gThreadFuncs)
FCFuncElement("InternalGetCurrentThread", GetThread)
FCFuncElement("SleepInternal", ThreadNative::Sleep)
FCFuncElement("Initialize", ThreadNative::Initialize)
FCFuncElement("SpinWaitInternal", ThreadNative::SpinWait)
FCFuncElement("GetCurrentThreadNative", ThreadNative::GetCurrentThread)
FCFuncElement("InternalFinalize", ThreadNative::Finalize)
FCFuncElement("get_IsAlive", ThreadNative::IsAlive)
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/qcallentrypoints.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ static const Entry s_QCall[] =
DllImportEntry(ThreadNative_GetCurrentOSThreadId)
DllImportEntry(ThreadNative_Abort)
DllImportEntry(ThreadNative_ResetAbort)
DllImportEntry(ThreadNative_SpinWait)
#ifdef TARGET_UNIX
DllImportEntry(WaitHandle_CorWaitOnePrioritizedNative)
#endif
Expand Down