AsyncEx icon indicating copy to clipboard operation
AsyncEx copied to clipboard

[Discussion] Return `ValueTask` instead of `AwaitableDisposable`?

Open BalassaMarton opened this issue 2 years ago • 5 comments

Methods returning AwaitableDisposable could return ValueTask instead.

Pros

  • Much faster and allocates less on the happy-path (when the lock is not already taken).

Cons

  • Massively breaking change due to the limitations of ValueTask.
  • Not compatible with older frameworks.

I'll submit an experimental PR just for the sake of discussion.

BalassaMarton avatar Oct 14 '22 20:10 BalassaMarton

Could the System.Threading.Tasks.Extensions nuget package be used to support older framworks.

modmynitro avatar Oct 27 '22 08:10 modmynitro

Could the System.Threading.Tasks.Extensions nuget package be used to support older framworks.

Yes, it should be possible to use ValueTask from System.Threading.Tasks.Extensions to support older frameworks.

Pretty late to the discussion, but this PR as a whole would be pretty interesting to expand on.

NotTsunami avatar Apr 09 '24 18:04 NotTsunami

Created a new experimental async lock implementation that supports fast, zero-alloc reentrancy: https://github.com/BalassaMarton/AsyncExtensions I'd be happy to see your feedback, especially from @StephenCleary

BalassaMarton avatar Apr 09 '24 21:04 BalassaMarton

@BalassaMarton I'm always skeptical of any re-entrant async locks. Does your solution hold up to the problems presented in this article? I even tried my hand at the problem myself, but abandoned the idea. (But I do have an alloc-free non-re-entrant async lock in my library.)

timcassell avatar Apr 09 '24 21:04 timcassell

@timcassell look at this example: https://github.com/BalassaMarton/AsyncExtensions?tab=readme-ov-file#example-recursive-async-locking-using-a-token The specific problem I was trying to solve: I had a bunch of short, specific methods that needed exclusive access to resources, but it was hard to follow which methods were doing the actual locking vs assuming the lock being already taken. With this approach, the method parameter effectively documents that it will asynchronously lock on the resource if it's not locked already:

async ValueTask OuterMethod(AsyncLockToken token = default)
{
    using (token = await mutex.LockAsync(token))
    {
        await InnerMethod(token);
    }
}

async ValueTask InnerMethod(AsyncLockToken token = default)
{
    using (await mutex.LockAsync(token))
    {
        // ...
    }
}

I even had a variation where you could name your AsyncLock objects and annotate any method that will lock on them like [LocksOn("lockName")] - and LockAsync would validate that methods in the current call stack are correctly annotated. This is of course all experimental, I'm not saying any of this is the way to do async locking.

BalassaMarton avatar Apr 12 '24 18:04 BalassaMarton