time: delay the cancellation of timers #7467
Conversation
ADD-SP
added
A-tokio
ADD-SP
force-pushed
the
add_sp/time-local-wheel
branch
11 times, most recently
from
c2d5790 to
d04c22f
Compare
ADD-SP
changed the title
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn reset_later_after_slot_starts() { |
There was a problem hiding this comment.
Note
This test was removed because it was testing the behavior of tokio::runtime::time::entry::reset, which is removed in this PR.
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn reset_earlier_after_slot_starts() { |
There was a problem hiding this comment.
Note
This test was removed because it was testing the behavior of tokio::runtime::time::entry::reset, which is removed in this PR.
| sleep(ms(20)).await; | ||
|
|
||
| assert!(queue.is_woken()); | ||
| assert_ready_some!(poll!(queue)); |
There was a problem hiding this comment.
Note
queue.reset_at resets inner Sleep, however, the new implementation will drop the inner timer and create a new one. So the waker will not be called, we have to poll manually.
| sleep(ms(20)).await; | ||
|
|
||
| assert!(queue.is_woken()); | ||
| assert_ready_some!(poll!(queue)); |
There was a problem hiding this comment.
Note
queue.reset_at resets inner Sleep, however, the new implementation will drop the inner timer and create a new one. So the waker will not be called, we have to poll manually.
| // runtime is shutting down | ||
| // OR waking up expired timers | ||
|
|
||
| // Track that a task was scheduled from **outside** of the runtime. | ||
| self.shared.scheduler_metrics.inc_remote_schedule_count(); | ||
|
|
||
| // Schedule the task | ||
| self.shared.inject.push(task); | ||
| self.driver.unpark(); |
There was a problem hiding this comment.
Note
Since the wheel is stored in the Core, the cx.core is None while waking up timers.
While we could store Option in Core, this would double the number of possible states in the local time subsystem, which isn't worth it.
I know this creates some lock contention on the inject queue, but it doesn't shows the regression on the benchmark, so I kept it for now. Feel free to share your thought and possible optimization.
ADD-SP
force-pushed
the
add_sp/time-local-wheel
branch
2 times, most recently
from
695eea8 to
e9255ee
Compare
| feature = "signal", | ||
| feature = "time", | ||
| ))] | ||
| pub(crate) use wake_list::WakeList; |
There was a problem hiding this comment.
Note
We no long use WakeList in the time subsystem.
…try::cancel_pointers` Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
| /// True if the driver is being shutdown. | ||
| is_shutdown: AtomicBool, | ||
|
|
||
| // When `true`, a call to `park_timeout` should immediately return and time | ||
| // should not advance. One reason for this to be `true` is if the task | ||
| // passed to `Runtime::block_on` called `task::yield_now()`. | ||
| // | ||
| // While it may look racy, it only has any effect when the clock is paused | ||
| // and pausing the clock is restricted to a single-threaded runtime. | ||
| #[cfg(feature = "test-util")] | ||
| did_wake: AtomicBool, |
There was a problem hiding this comment.
Moving these atomics into their own Arc is best avoided. That's an allocation per integer, and they may be stored far away from each other hurting cache locality.
There was a problem hiding this comment.
Let me revisit the shutdown process and relevant dataflows, will come back later.
There was a problem hiding this comment.
Improved, details are attached to https://gist.github.com/ADD-SP/4037e25256b8dc8a4956962415de2356#file-d-propagate-shutdown-notification-md.
Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
Signed-off-by: ADD-SP <[email protected]>
Review guide
Design document
See https://gist.github.com/ADD-SP/4037e25256b8dc8a4956962415de2356.
Benchmarks
See https://gist.github.com/ADD-SP/4037e25256b8dc8a4956962415de2356.