mscorlib cache padding for 128 byte lines #13102
Conversation
|
@dotnet/arm64-contrib @stephentoub @kouvel PTAL coreclr equivalent of dotnet/corefx#22724 |
| @@ -1111,10 +1111,10 @@ internal struct Slot | |||
|
|
|||
| /// <summary>Padded head and tail indices, to avoid false sharing between producers and consumers.</summary> | |||
| [DebuggerDisplay("Head = {Head}, Tail = {Tail}")] | |||
| [StructLayout(LayoutKind.Explicit, Size = 192)] // padding before/between/after fields based on typical cache line size of 64 | |||
| [StructLayout(LayoutKind.Explicit, Size = 384)] // padding before/between/after fields based on worst case cache line size of 128 | |||
There was a problem hiding this comment.
It seems we may want to make this change ARM64 specific (put it under #if ARM64)
sdmaclea
force-pushed
the
PR-ConcurrentQueue-128
branch
from
31b21b2 to
a946aab
Compare
| @@ -1109,12 +1109,23 @@ internal struct Slot | |||
| } | |||
| } | |||
|
|
|||
| /// <summary>A placeholder class for common padding constants and eventually routines.</summary> | |||
| internal static class PaddedHelper | |||
There was a problem hiding this comment.
We now have two places where we define the cache line size (at the other place, we use just the 128 bytes for all platforms). I wonder if we should unify it and put it e.g. to System.Environment.
@jkotas, would the System.Environment be the right place?
There was a problem hiding this comment.
Environment is a dumping ground for everything. It may be better to keep it as a small type with a constant. It does not really matter because of it has to be internal in the current shape, and thus we can change it anytime.
There was a problem hiding this comment.
src/mscorlib/src/Internal/CacheHelpers.cs
Sounds good to me.
sdmaclea
force-pushed
the
PR-ConcurrentQueue-128
branch
from
a946aab to
9641c9d
Compare
| @@ -356,6 +356,7 @@ | |||
| <Compile Include="$(BclSourcesRoot)\Internal\Runtime\Augments\EnvironmentAugments.cs" /> | |||
| <Compile Include="$(BclSourcesRoot)\Internal\Runtime\Augments\RuntimeThread.cs" /> | |||
| <Compile Include="$(BclSourcesRoot)\Internal\Console.cs" /> | |||
| <Compile Include="$(BclSourcesRoot)\Internal\Padding.cs" /> | |||
There was a problem hiding this comment.
I used the filename Padding.cs because I noticed corefx used that name for the equivalent file.
| [StructLayout(LayoutKind.Explicit, Size = PaddingHelpers.CACHE_LINE_SIZE - sizeof(int))] | ||
| internal struct PaddingFor32 | ||
| { | ||
| } |
There was a problem hiding this comment.
I left these structure pretty much intact because that matches corefx.
|
@dotnet-bot test OSX10.12 x64 Checked Build and Test please |
|
@jkotas Thanks. Tests are clean. This can be merged. |
No description provided.