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

Implement thread park(), unpark(), and panicking() #75

Open
kvark opened this issue Aug 16, 2022 · 1 comment
Open

Implement thread park(), unpark(), and panicking() #75

kvark opened this issue Aug 16, 2022 · 1 comment

Comments

@kvark
Copy link

kvark commented Aug 16, 2022

Already a TODO in code:

// TODO: Implement park(), unpark()

This would help making shuttle::thread to be an in-place substitute to std::thread.

jamesbornholt added a commit to jamesbornholt/shuttle that referenced this issue Aug 19, 2022
The only thing we actually care about here is `hint::spin_loop`, which
we just treat the same way as `yield_now`.

Part of awslabs#75.
@jamesbornholt
Copy link
Member

#77 will add park/unpark, and #78 will add hint::spin_loop, which I think you might also need. Those two PRs plus this diff to choir was enough to get your queue::shuttle test running (and passing!):

diff --git a/src/queue.rs b/src/queue.rs
index e9b677d..cfcbb42 100644
--- a/src/queue.rs
+++ b/src/queue.rs
@@ -1,8 +1,8 @@
 #[cfg(shuttle)]
-use shuttle::{sync, thread};
-use std::{cell::UnsafeCell, hint, mem};
+use shuttle::{sync, hint, thread};
+use std::{cell::UnsafeCell, mem};
 #[cfg(not(shuttle))]
-use std::{sync, thread};
+use std::{sync, hint, thread};
 
 use self::sync::atomic::{AtomicUsize, Ordering};
 
@@ -194,7 +194,7 @@ impl<T> Drop for Queue<T> {
 #[cfg(shuttle)]
 #[test]
 fn shuttle() {
-    shuttle::check_random(test_barrage, 10000);
+    shuttle::check_pct(test_barrage, 3, 10000);
 }
 
 #[test]
@@ -219,7 +219,7 @@ fn test_smoke() {
 #[cfg_attr(not(shuttle), test)]
 fn test_barrage() {
     const NUM_THREADS: usize = if cfg!(miri) { 2 } else { 8 };
-    const NUM_ELEMENTS: usize = if cfg!(miri) { 1 << 7 } else { 1 << 16 };
+    const NUM_ELEMENTS: usize = if cfg!(miri) || cfg!(shuttle) { 1 << 7 } else { 1 << 16 };
     let sq = sync::Arc::new(Queue::new(NUM_ELEMENTS));
     let mut handles = Vec::new();
 

Note that I switched the test from check_random to check_pct. Our current implementation of check_random isn't good at spin loops, since in some sense you have to "get lucky" to break out of the infinite loop enough times to make progress. PCT is another randomized scheduler that is better at being able to escape spin loops, as long as you use the spin loop hints. The extra parameter 3 to check_pct is an upper bound on how many preemptions the scheduler will explore, in the same vein as loom's preemption_bound config.

The NUM_ELEMENTS change is only to make the test fit without our default bound on the number of steps a test can take. If you wanted to keep it as is, you could bump up that bound instead:

#[cfg(shuttle)]
#[test]
fn shuttle() {
    let scheduler = PctScheduler::new(3, 10000);
    let runner = Runner::new(
        scheduler,
        shuttle::Config {
            max_steps: shuttle::MaxSteps::FailAfter(100_000_000),
            ..Default::default()
        },
    );
    runner.run(test_barrage);
}

std::thread::panicking is a little trickier for us to implement right now, because of the somewhat janky way we use a panic hook to intercept and record failing tests. I need to think a bit more about a good way to make this work.

jamesbornholt added a commit to jamesbornholt/shuttle that referenced this issue Aug 23, 2022
The only thing we actually care about here is `hint::spin_loop`, which
we just treat the same way as `yield_now`.

Part of awslabs#75.
jamesbornholt added a commit that referenced this issue Aug 23, 2022
The only thing we actually care about here is `hint::spin_loop`, which
we just treat the same way as `yield_now`.

Part of #75.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants