-
Notifications
You must be signed in to change notification settings - Fork 501
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
Document implicit yield in install() per #1105 #1107
Conversation
rayon-core/src/thread_pool/mod.rs
Outdated
/// `install()` implicitly calls [`ThreadPool::yield_now()`], | ||
/// potentially scheduling another task to run on the current thread. For | ||
/// example: |
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.
I would like this to clarify the context a bit more, something like:
/// `install()` implicitly calls [`ThreadPool::yield_now()`], | |
/// potentially scheduling another task to run on the current thread. For | |
/// example: | |
/// If the current thread is part of a different thread pool, it will try to | |
/// keep busy while the `op` completes in its target pool, similar to | |
/// calling [`ThreadPool::yield_now()`] in a loop. Therefore, it may | |
/// potentially schedule other tasks to run on the current thread in the | |
/// meantime. For example: |
Does that make sense to you?
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.
Yes, changed to your phrasing via force push.
Thanks! |
rayon-core/src/thread_pool/mod.rs
Outdated
/// fn main() { | ||
/// rayon::ThreadPoolBuilder::new().num_threads(1).build_global().unwrap(); | ||
/// let pool = rayon::ThreadPoolBuilder::default().build().unwrap(); | ||
/// (0..3).into_par_iter().for_each(|_| { |
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.
Oh, we can't use APIs from rayon
proper while testing rayon-core
. Maybe you can make the example with join
instead?
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.
Foiled! Rewritten to use join()
as requested.
Add documentation to
rayon::ThreadPool::install()
warning about implicit yield behavior, as discussed in #1105.