Hoist volatile variable's access outside the loop #34225
Conversation
`_tables` is a volatile object and it has `_countPerLock` volatile field. On ARM64, JIT generates memory barrier instruction "dmb" for every volatile variable access which could be expensive. This PR caches the volatile variables outside the loop and use cached local variables instead. Fixes: dotnet#34198
...ries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
Outdated
Show resolved
Hide resolved
...ries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
Outdated
Show resolved
Hide resolved
...ries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
Outdated
Show resolved
Hide resolved
|
Test failures are related to #28553. |
|
To satisfy my curiosity ... Do I understand correctly that semantics of "volatile" in C# does /not/ require compiler to preserve the number of reads and writes of volatile variables? (This is unlike C/C++ where language standard prevents a similar optimisation.) |
|
@maxim-kuvyrkov I'm also a bit irritated - I also thought the whole point of |
Why do you think it doesn't? |
Because a statement hoisted from a loop will execute once, instead of the number of loop iterations. |
Did you look at the change? It's not changing the compiler/JIT. it's manually moving a read out of a specific loop because it's not necessary to re-read this particular field on every iteration of this particular loop, but as a volatile it would be. |
|
@stephentoub , thanks, this clears things up. No, I didn't read the code and from [1] I assumed that this change is for JIT. So C#'s "volatile" semantics mandates unchanged read/write count like C/C++? [1] https://devblogs.microsoft.com/dotnet/arm64-performance-in-net-5/ |
ghost
locked as resolved and limited conversation to collaborators
_tablesis a volatile object and it has_countPerLockvolatile field. On ARM64, JIT generates memory barrier instruction "dmb" for every volatile variable access which could be expensive. This PR caches the volatile variables outside the loop and use cached local variables instead. All the places where I have hoisted the variable access are guarded byAcquireAllLocks()method.Fixes: #34198