-
-
Notifications
You must be signed in to change notification settings - Fork 636
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
Port the bulk of the process_execution crate to async/await #9676
Port the bulk of the process_execution crate to async/await #9676
Conversation
[ci skip-jvm-tests]
# Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
@@ -24,11 +24,11 @@ | |||
#![allow(clippy::new_without_default, clippy::new_ret_no_self)] | |||
// Arc<Mutex> can be more clear than needing to grok Orderings: | |||
#![allow(clippy::mutex_atomic)] | |||
#![type_length_limit = "7000100"] | |||
#![type_length_limit = "32185118"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? It looks like we're now 32x times the default: https://doc.rust-lang.org/reference/attributes/limits.html#the-type_length_limit-attribute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async/await generates very large per-method types that describe an anonymous struct holding the state machine for the run... luckily you never see them (in error messages, for example). But they're there.
I went down a bit of a rabbit hole looking for something clearly stating that this is fine, but didn't find anything that definitive. The best I could find is that they're continuing to try and improve it: rust-lang/rust#46477
} | ||
}) | ||
.and_then( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow. What a difference 👏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The final boss, heh.
Problem
We're planning to instrument process execution with additional workunits, but it can be challenging to identify critical sections in the presence of combinators.
Solution
async/await for the bulk of the process_execution crate.