Use new Interlocked.Exchange/CompareExchange overloads that support bool values #4585
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| #if NET9_0_OR_GREATER | ||
| using TBool = System.Boolean; | ||
| #else | ||
| using TBool = System.Int32; | ||
| #endif | ||
|
|
||
| namespace Sentry.Internal; | ||
|
|
||
| internal struct InterlockedBoolean | ||
| { | ||
| private volatile TBool _value; | ||
|
|
||
| [Browsable(false)] | ||
| internal TBool ValueForTests => _value; | ||
|
|
||
| #if NET9_0_OR_GREATER | ||
| private const TBool True = true; | ||
| private const TBool False = false; | ||
| #else | ||
| private const TBool True = 1; | ||
| private const TBool False = 0; | ||
| #endif | ||
|
|
||
| public InterlockedBoolean() { } | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public InterlockedBoolean(bool value) { _value = value ? True : False; } | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static implicit operator bool(InterlockedBoolean @this) => (@this._value != False); | ||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static implicit operator InterlockedBoolean(bool @this) => new InterlockedBoolean(@this); | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public bool Exchange(bool newValue) | ||
| { | ||
| TBool localNewValue = newValue ? True : False; | ||
|
|
||
| TBool localReturnValue = Interlocked.Exchange(ref _value, localNewValue); | ||
|
|
||
| return (localReturnValue != False); | ||
| } | ||
|
Comment on lines
+35
to
+42
Member
There was a problem hiding this comment. suggestion: potentially more efficient implementation ... candidate for a follow-up PR I am wondering if we can make the implementation for Something like: // ...
#if NET9_0_OR_GREATER
internal volatile bool _value;
#else
internal volatile int _value;
#endif
// ...
public bool Exchange(bool newValue)
{
#if NET9_0_OR_GREATER
return Interlocked.Exchange(ref _value, newValue);
#else
int localNewValue = newValue ? 1 : 0;
int localReturnValue = Interlocked.Exchange(ref _value, localNewValue);
return (localReturnValue != 0);
#endif
}
// ...This way we wouldn't need any However, I am quite happy to having this as a follow-up PR, if you'd like ... as the current implementation is functional and this change is about style and performance.
Contributor
Author
There was a problem hiding this comment. I'm not able to predict exactly what it'll do, but I actually wouldn't be surprised if the JIT's code optimization just treated the same storage as Now I'm curious 🙂 I might do some poking just to see.
Contributor
Author
There was a problem hiding this comment. Okay, I took a peek at what the JIT is doing. In Source code: Compiled code: So that's kind of disappointing but also not a huge surprise. 🙂 Interestingly, the return value isn't quite so horrifically inefficient: Source code, in which localReturnValue is a bool because I'm running in .NET 9: Compiled code: So, in the return code, it recognized that Anyway, that's the Source code: Compiled code: As for the return, well, that gets interesting :-) The Source code: Compiled code: So how does that compare against just a direct call to Source code: Compiled code: So basically identical to the tail end of the generic implementation. It's not perfectly-optimized, but it's pretty good. What does this mean for performance? It means the difference between being able to do It appears that that extra lead-up is almost entirely inconsequential. I made a program to benchmark it, and very consistently, the "slow" path is only 1.2% slower than the "fast" path. I'll polish up the benchmark code and throw it up into a repo somewhere, because maybe I made a silly mistake somewhere. 🙂
Contributor
Author
There was a problem hiding this comment. Even in Here's the benchmark: https://github.com/logiclrd/InterlockedExchangePerformanceTest |
||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public bool CompareExchange(bool value, bool comparand) | ||
| { | ||
| TBool localValue = value ? True : False; | ||
| TBool localComparand = comparand ? True : False; | ||
|
|
||
| TBool localReturnValue = Interlocked.CompareExchange(ref _value, localValue, localComparand); | ||
|
|
||
| return (localReturnValue != False); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.