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
Doc improvements. #71
Conversation
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.
Thank you for makeing it nice.
@@ -216,7 +224,7 @@ impl ThreadPool { | |||
name: name, | |||
job_receiver: Mutex::new(rx), | |||
empty_condvar: Condvar::new(), | |||
empty_trigger: Mutex::new( () ), | |||
empty_trigger: Mutex::new(()), |
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 have left that for readability
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.
cargo fmt
did this, but I can revert it
lib.rs
Outdated
/// pool.execute(|| println("hello")); | ||
/// pool.execute(|| println("world")); | ||
/// pool.execute(|| println("foo")); | ||
/// pool.execute(|| println("bar")); |
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 println
does not inwoke the macro. -> !
/// }); | ||
/// } | ||
/// | ||
/// sleep(Duration::from_secs(1)); // wait for threads to start |
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.
cool 👍
lib.rs
Outdated
/// pool.execute(move || { | ||
/// panic!() | ||
/// if n % 2 == 0 { | ||
/// panic!() |
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.
Maybe add a comment like "simulating panic" so the test output does not look that grimm
/// | ||
/// // Decrease thread capacity of the pool | ||
/// // No active threads are killed | ||
/// pool.set_num_threads(4); |
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.
👍
pub fn set_num_threads(&mut self, num_threads: usize) { | ||
assert!(num_threads >= 1); | ||
let prev_num_threads = self.shared_data.max_thread_count.swap(num_threads, Ordering::Release); | ||
let prev_num_threads = self.shared_data.max_thread_count.swap( |
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 wonder what rule you would like to implement for the linewrap. Is it the length of the line?
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'm not sure, cargo fmt
wrapped this and I'm not sure what its rules are
Well if the rule is "what ever |
@@ -322,8 +414,7 @@ impl ThreadPool { | |||
} | |||
} | |||
|
|||
/// Block the current thread until all jobs in the pool are completed. | |||
/// Many threads can wait for a pool to finish concurrently. | |||
/// Block the current thread until all jobs in the pool have been executed. |
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 add a text like this:
A pool can be joined repeatedly.
Calling the join method on an empty pool will cause it to return immediately.
The join method may be used from multiple threads at the same time. maybe link to test
Warning
Joining the pool from within the pool will cause a deadlock.
This behavior is considered safe.
bors r+ |
No description provided.