ImTools
ImTools copied to clipboard
Review of public Ref(external count, Thread.Yield, error return)
Did some view onto, hope you find some useful.
https://github.com/dadhi/ImTools/blob/56b7f53a784c3843e4299658592d7b9340153fbc/src/ImTools/ImTools.cs#L534
Also it is public, it may deliver some caveats to possible users:
- Spin probably should yield for better CPU https://referencesource.microsoft.com/#mscorlib/system/threading/thread.cs,dd960cd58d3d20c1,references
- Default counter should in public API, not private hidden surprise in production.
- Consider return error(value tuple, option, or by ref return) result from Swap instead of throwing exception. Retry exceed could be normal condition on low level.
- Reproduce in test retry count reached https://github.com/dadhi/ImTools/blob/56b7f53a784c3843e4299658592d7b9340153fbc/src/ImTools.UnitTests/RefTests.cs#L7 via one fast and one slow getNewValue. Until getNewValue is hanged and if Thread.Yield used I guess retry could be made infinite by default.
- Consider make it more private-internal if possible.
- T is class. Consider adding know primiteves swap like Interlocked for long int etc. https://github.com/dadhi/ImTools/blob/56b7f53a784c3843e4299658592d7b9340153fbc/src/ImTools/ImTools.cs#L524
- https://github.com/dadhi/ImTools/blob/56b7f53a784c3843e4299658592d7b9340153fbc/src/ImTools/ImTools.cs#L489 consider document
reference equals
instead ofequals
(to be crystal clear). - CompareAndSwap or other like name may be better name https://github.com/dadhi/ImTools/blob/56b7f53a784c3843e4299658592d7b9340153fbc/src/ImTools/ImTools.cs#L497
- https://github.com/dadhi/ImTools/blob/56b7f53a784c3843e4299658592d7b9340153fbc/src/ImTools/ImTools.cs#L498 does compares by reference, but returns by bool depending on possible
==
https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/equality-comparison-operator. Same seems here https://github.com/dadhi/ImTools/blob/56b7f53a784c3843e4299658592d7b9340153fbc/src/ImTools/ImTools.cs#L531 - Consider document usage scenarios, i.e. it seems not for long running operations, not ordered operations (while does not orders getNewValue delegates).
- Inspire https://www.cl.cam.ac.uk/techreports/UCAM-CL-TR-579.pdf :) and think of laptop Intel with 12 cores or Ryzen with 32 cores https://en.wikipedia.org/wiki/Ryzen (these are commodities now)
Found similar code in .NET source
public static X TryEnterWriteLock(ref X value, TimeSpan timeout)
{
// https://github.com/dotnet/corefx/blob/3b24c535852d19274362ad3dbc75e932b7d41766/src/Common/src/CoreLib/System/Threading/ReaderWriterLockSlim.cs#L233
var tracker = new TimeoutTracker(timeout);
if (tracker.IsExpired)
{
throw new TimeoutException("Some other lock is hold for too long time");
}
I guess throw via Thrower may save some performance of spin. Could check that. Example is https://github.com/jtmueller/Collections.Pooled/issues/8
These people are not afraid of timeout https://github.com/scalaz/scalaz-zio/blob/master/core/shared/src/main/scala/scalaz/zio/Ref.scala#L42. And use function. May be this Ref should to.
Another Ref
https://github.com/atifaziz/Interlocker/issues