Skip to content

[TEST] Async2 with variants turned off #114082

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 197 commits into from
Closed

[TEST] Async2 with variants turned off #114082

wants to merge 197 commits into from

Conversation

VSadov
Copy link
Member

@VSadov VSadov commented Mar 31, 2025

Test-only PR.
A baseline run with Async2 variants turned off in VM

davidwrighton and others added 30 commits November 16, 2023 14:23
@davidwrighton
More updates
0beb3f0
@davidwrighton
more tweaks
783754d
@davidwrighton
It builds
8c43a9a
@davidwrighton
ItCompilesButConvertingFromAsyncVariantsIsntRight
545509c
@davidwrighton
It builds and it should work... but doesn't
20eb76e
@davidwrighton
Oops pMethod can be NULL
ad0bd71
@davidwrighton
We are able to load Task<T> methods which are virtual again, next to …
8d416cb 
…actually write some "async" and start breaking things again
@davidwrighton
Add some simple benchmarks of the existing situation
0fbd60c
@davidwrighton
Add Test case for synchronous behavior
3270fdf
@davidwrighton
Now we should be forcing transient lookup which wont' work
84ae54d
@davidwrighton
Fix method classification for async2 methods
84f7148
@davidwrighton
We are now triggering the transient IL compilation for the thunks
51e2011
@davidwrighton
Calling Sample Works! Now to implement an actual async implementation…
6fc166a 
… and see if suspension works

NOTE: No methods with generic parameters will work with the current implementation, generics may require a bunch of infrastructure work, separate from the async work
@davidwrighton
WithYield runtime task test
0b10933
@davidwrighton
- Add async to Task stub
34a2219 
- Public apis for directly awaiting from async2 code
- Update test case to use the new apis and now the code is lovely and simple
@davidwrighton
Runtime handled tasks markdown for experiment
38b2087
@davidwrighton
Writeup managed side of runtime driven suspension. Need to write deta…
b7b00de 
…ils into doc, but managed code may have the right logic :)
@davidwrighton
Add the logic to try to flow ExecutionContext and SynchronizationCont…
8353bd8 
…ext as appropriate
@davidwrighton
It compiles, but doesn't link
2aac618
@davidwrighton
C++ portion builds
0e82446
@davidwrighton
In theory, this should work?
718aa39
@davidwrighton
native part builds
6d1e7b7
@VSadov @davidwrighton
Merged PR 34323: make things build
d20bb17 
make things build
@davidwrighton
It successfully captures... and unwind doesn't quite work
aee9b9f
@davidwrighton
Basic unwind support mostly works. Expected that it crashes unpredict…
df91ac6 
…ably in the presence of Suspend operations, but other stuff seems to work. Now to rework Task generation to avoid multithreaded races
@davidwrighton
It runs? Doesn't handle return value correctly, but runs for the righ…
56178c3 
…t number of iterations, I think
@davidwrighton
Free memory properly
52f272b
@VSadov @davidwrighton
Merged PR 34523: remove DEFINE_METHOD for no longer existing methods
204633e 
remove DEFINE_METHOD for no longer existing methods
@davidwrighton
Fix returns within async2 tasks... returning value to outer task is s…
f30fa3f 
…till not working
@davidwrighton
Set the return value on output from this whole thing! The example works!
19b5c99
@VSadov VSadov added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it area-VM-coreclr runtime-async labels Mar 31, 2025
@ghost
Copy link

ghost commented Mar 31, 2025

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

1 similar comment
@ghost
Copy link

ghost commented Mar 31, 2025

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@kg kg mentioned this pull request Apr 3, 2025
Closed
This was referenced Mar 31, 2025
Open
Open
Closed
Closed
Closed
Closed
@VSadov VSadov closed this Apr 15, 2025
@github-actions github-actions bot locked and limited conversation to collaborators May 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky will be requested when the pull request is marked ready for review MichalStrehovsky is a code owner

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees

@VSadov VSadov

Labels
area-VM-coreclr new-api-needs-documentation NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it runtime-async
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@VSadov @davidwrighton @jakobbotsch @IS4Code @danmoseley