Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mono] Set thread suspend default in mono.proj; default to hybrid #46873

Merged
merged 4 commits into from
Jan 16, 2021

Conversation

lambdageek
Copy link
Member

Instead of detecting in cmake, make a decision in mono.proj

@lambdageek
Copy link
Member Author

This is, in part, motivated by #44806 - MBR needs hybrid (or full coop) suspend in order to be able to inspect interpreter frames. So we will need hybrid suspend on the mobile platforms.

Previously hybrid suspend was already well tested on Linux and OSX amd64. And apparently in dotnet/runtime we already used it on Android, too.

If it is not feasible to identify and fix all hybrid suspend issues, we could set the default back to preemptive on affected platforms.

(For MBR we would then launch hot reload sessions with hybrid suspend set via environment variables).

This also sets "hybrid" as the suspend mechanism on wasm, although in single threaded operation it doesn't make a difference.

Instead of detecting in cmake, make a decision in mono.proj
Copy link
Contributor

@CoffeeFlux CoffeeFlux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope you're ready to start putting out fires when the CI failures roll in 😄

src/mono/CMakeLists.txt Show resolved Hide resolved
@lambdageek
Copy link
Member Author

wasm failure looks related. android and windows x64 also possibly related.

It isn't really preemptive, it just turns off safepoint code in the AOT
compiler.  This is fine for single threaded.
@lambdageek lambdageek force-pushed the switch-to-hybrid-suspend branch from 59f94c7 to 48f8e55 Compare January 15, 2021 19:32
@lambdageek
Copy link
Member Author

Defaulting wasm to "preemptive". Otherwise it was asserting when trying to insert safepoints in AOT code. Wasm isn't really preemptive but we don't need safepoints in single-threaded. (and this is fine for hot reload, too, because the LMF has all the managed interpreted frames that we care about - we know the thread is calling the hot reload icall)

@lambdageek
Copy link
Member Author

okay, let's do this, I guess. Looking forward to all the new crash reports over the weekend.

@lambdageek lambdageek merged commit c31d8ae into dotnet:master Jan 16, 2021
@marek-safar
Copy link
Contributor

Otherwise it was asserting when trying to insert safepoints in AOT code.

Could you create a tracking issue for that if there is not one already?

@lambdageek
Copy link
Member Author

Ok, opened #47080

@lambdageek lambdageek deleted the switch-to-hybrid-suspend branch January 20, 2021 00:19
@ghost ghost locked as resolved and limited conversation to collaborators Feb 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants