ThreadPoolWorkQueue.numOutstandingThreadRequests is not being padded as requested, despite the explicit sequential layout

[kouvel]

Jitted code generated for ThreadPoolWorkQueue.EnsureThreadRequested shows that the offset to the field is not cache-line separated from prior fields as requested.

Replacing the field declaration with the following seems to work as expected:

 [StructLayout(LayoutKind.Sequential)]
 private struct CacheLineSeparated
 {
 private readonly Internal.PaddingFor32 pad1;

 public volatile int numOutstandingThreadRequests;

 private readonly Internal.PaddingFor32 pad2;
 }

 private CacheLineSeparated _separated;

[stephentoub]

as requested

It's defined on a class, which will have an auto layout. The fields can be re-arranged however the runtime sees fit.

[kouvel]

The class has an attribute for sequential layout here:

runtime/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.cs

Lines 25 to 26 in 3495f1a

	[StructLayout(LayoutKind.Sequential)] // enforce layout so that padding reduces false sharing
	internal sealed class ThreadPoolWorkQueue

The docs seem to suggest that it should work on a class, so I'm still a bit confused what's special about this case:
https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.structlayoutattribute?view=netcore-3.1

[stephentoub]

"For non-blittable types, it controls the layout when the class or structure is marshaled to unmanaged code, but does not control the layout in managed memory."

[kouvel]

Ah ok thanks, missed that part. I'll go ahead and change it to the above code.

[stephentoub]

Thanks. Was false sharing on this causing a measurable slowdown? That'd be a nice "free" win :)

[kouvel]

It was difficult to measure, it was very close, but I was seeing that it helps a tiny bit along with some other change I was testing with to reduce the number of threads requested. May also depend on the machine and test. At least I didn't see a regression in my tests with the fix.