Don't runtime-await Delegate.Invoke methods #122491
Conversation
This would be doubly inefficient because: * The method pointed to by the delegate is guaranteed not runtime-async (so we'd go through up to two thunks), and * the variant of `Invoke` method is no longer _the_ `Delegate.Invoke` method that can be reported as `CORINFO_FLG_DELEGATE_INVOKE` to RyuJIT (needs to be invoked as a regular method instead of the efficient delegate invoke sequence). CoreCLR does this in methodtablebuilder.cs where we consider all methods on delegates as `MethodReturnKind::NormalMethod`.
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
There was a problem hiding this comment.
Pull request overview
This PR fixes an inefficiency where Delegate.Invoke methods were being runtime-awaited, which is problematic because the method pointed to by the delegate is guaranteed not to be runtime-async. The fix adds checks in two compiler locations to prevent async variants from being used for delegate types, and includes a test to validate the fix.
Key changes:
- Added delegate type check in ILImporter.Scanner to skip async variant generation
- Added delegate type check in CorInfoImpl to prevent async variant resolution
- Added comprehensive test covering both compiler-async and runtime-async delegate scenarios
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/tests/async/delegates/delegates.csproj | New test project file for delegate async testing |
| src/tests/async/delegates/delegates.cs | New test validating delegate invocation with both compiler and runtime async methods |
| src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs | Added check to skip async variant when method's owning type is a delegate |
| src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | Added check with explanatory comment to prevent async variant resolution for delegate methods |
Hmm, interesting. It means that |
cc @VSadov |
|
Virtual/interface dispatch is the only kind of indirection through which we can propagate async right now. Method pointers do not have practical solutions. (exposing async call convention to the user is probably too big of a hammer). Delegate might be doable in some scenarios. They are just hard as they have all kind of things - argument shuffling, multicast, BeginInvoke, ... etc. Also the most common and interesting case |
I wonder what we do with Task-returning vararg methods... Perhaps should not be classified as ReturnsTaskOrValueTask either. |
I think this is something we will be optimizing via PGO and GDV (and the corresponding "whole-program view" optimizations on the NAOT side). |
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
This is doubly inefficient because:
Invokemethod is no longer theDelegate.Invokemethod that can be reported asCORINFO_FLG_DELEGATE_INVOKEto RyuJIT (needs to be invoked as a regular method instead of the efficient delegate invoke sequence).CoreCLR does this in methodtablebuilder.cs where we consider all methods on delegates as
MethodReturnKind::NormalMethod.Also adding a test because it looks like we don't have one.
Cc @dotnet/ilc-contrib