- Notifications
You must be signed in to change notification settings - Fork5.2k
Use Task.WaitAsync in SemaphoreSlim.WaitAsync#55262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
stephentoub commentedJul 7, 2021
| Method | Toolchain | Mean | Ratio | Allocated |
|---|---|---|---|---|
| WithCT | \main\CoreRun.exe | 1.103 us | 1.00 | 496 B |
| WithCT | \pr\CoreRun.exe | 1.032 us | 0.94 | 440 B |
| WithTimeout | \main\CoreRun.exe | 1.492 us | 1.00 | 888 B |
| WithTimeout | \pr\CoreRun.exe | 1.103 us | 0.74 | 536 B |
| WithCTandTimeout | \main\CoreRun.exe | 1.589 us | 1.00 | 904 B |
| WithCTandTimeout | \pr\CoreRun.exe | 1.200 us | 0.75 | 536 B |
ghost commentedJul 7, 2021
Tagging subscribers to this area:@mangod9 Issue Details
usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Diagnosers;usingBenchmarkDotNet.Running;usingSystem;usingSystem.Threading.Tasks;usingSystem.Threading;[MemoryDiagnoser]publicclassProgram{publicstaticvoidMain(string[]args)=>BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);privateSemaphoreSlim_sem=newSemaphoreSlim(0,1);privateCancellationTokenSource_cts=newCancellationTokenSource();[Benchmark]publicTaskWithCT(){Taskt=_sem.WaitAsync(_cts.Token);_sem.Release();returnt;}[Benchmark]publicTaskWithTimeout(){Taskt=_sem.WaitAsync(TimeSpan.FromMinutes(1));_sem.Release();returnt;}[Benchmark]publicTaskWithCTandTimeout(){Taskt=_sem.WaitAsync(TimeSpan.FromMinutes(1),_cts.Token);_sem.Release();returnt;}}
|
mangod9 commentedJul 7, 2021
Appears that there are some test failures though. |
stephentoub commentedJul 7, 2021
Yes, there's one test deterministically failing. I know why. What I need to figure out is why it ever passed prior to this :-) |
The Cancel_WaitAsync_ContinuationInvokedAsynchronously test was failing,highlighting that we were no longer invoking the continuationasynchronously from the Cancel call. But in fact we were incompletelydoing so in the past, such that we'd only force that asynchrony if notimeout was provided... if both a timeout and a token were provided,then we wouldn't. I've enhanced the test to validate both cases, andmade sure we now pass.
davidfowl commentedJul 8, 2021
Why not Task.Yield()? Do you care about avoiding the sync context? |
stephentoub commentedJul 8, 2021
Always :) |
davidfowl commentedJul 8, 2021
What about the extra delegate allocation tho 🙃 |
stephentoub commentedJul 8, 2021
That occurs only if cancellation is requested? I'm not worried about it :) |