-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
runtime: avoid unnecessary polling of block_on future #3582
Changes from 1 commit
ad9afd6
120ec47
37665a1
1d1d924
4eff7dc
6d772e9
7ffe2b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,8 @@ use std::cell::RefCell; | |
use std::collections::VecDeque; | ||
use std::fmt; | ||
use std::future::Future; | ||
use std::sync::atomic::AtomicBool; | ||
use std::sync::atomic::Ordering::{Acquire, Release}; | ||
use std::sync::Arc; | ||
use std::task::Poll::{Pending, Ready}; | ||
use std::time::Duration; | ||
|
@@ -70,6 +72,9 @@ struct Shared { | |
|
||
/// Unpark the blocked thread | ||
unpark: Box<dyn Unpark>, | ||
|
||
// indicates whether the blocked on thread was woken | ||
woken: AtomicBool, | ||
Comment on lines
+76
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if we have multiple calls to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Darksonn the field is set to false in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you implement the loom test I have mentioned below, that will answer this question. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is additional synchronization that ensures only one concurrent blocker hits this bool. |
||
} | ||
|
||
/// Thread-local context. | ||
|
@@ -101,6 +106,7 @@ impl<P: Park> BasicScheduler<P> { | |
shared: Arc::new(Shared { | ||
queue: Mutex::new(VecDeque::with_capacity(INITIAL_CAPACITY)), | ||
unpark: unpark as Box<dyn Unpark>, | ||
woken: AtomicBool::new(false), | ||
}), | ||
}; | ||
|
||
|
@@ -210,8 +216,10 @@ impl<P: Park> Inner<P> { | |
// Park until the thread is signaled | ||
scheduler.park.park().ok().expect("failed to park"); | ||
|
||
// Try polling the `block_on` future next | ||
continue 'outer; | ||
if scheduler.spawner.was_woken() { | ||
// Try polling the `block_on` future next if it was woken | ||
continue 'outer; | ||
} | ||
} | ||
} | ||
} | ||
|
@@ -329,8 +337,14 @@ impl Spawner { | |
} | ||
|
||
fn waker_ref(&self) -> WakerRef<'_> { | ||
// clear the woken bit | ||
self.shared.woken.store(false, Release); | ||
waker_ref(&self.shared) | ||
} | ||
|
||
fn was_woken(&self) -> bool { | ||
self.shared.woken.load(Acquire) | ||
} | ||
} | ||
|
||
impl fmt::Debug for Spawner { | ||
|
@@ -384,6 +398,7 @@ impl Wake for Shared { | |
|
||
/// Wake by reference | ||
fn wake_by_ref(arc_self: &Arc<Self>) { | ||
arc_self.woken.store(true, Release); | ||
arc_self.unpark.unpark(); | ||
} | ||
} | ||
|
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.
Can we use
crate::loom::sync::atomic::AtomicBool
here?