Unnecessary RwLock in time driver

[matiu2]

Version
1.83.0

Platform
Linux matiu-deskctop 6.11.11-1-MANJARO #1 SMP PREEMPT_DYNAMIC Thu, 05 Dec 2024 16:26:44 +0000 x86_64 GNU/Linux

Description
It all started with this forum post.

In short with 10k tasks, all waiting 15 msecs then incrementing an atomic int, there was a lot of mutex contention from the time driver.

The time driver "wheels" are behind a RwLock, but from what I can tell, it never needs writing. There's only one place in the code where the write lock is obtained (in the thread park method), but from what I can tell it doesn't need to be.

I made this patch in my local checkout of tokio, and ran the benchmark, and everything was fine. I also ran:

cargo +nightly test --features full

.. and everything was fine there too.

diff --git a/tokio/src/runtime/time/mod.rs b/tokio/src/runtime/time/mod.rs
index 56e0ba6..884b272 100644
--- a/tokio/src/runtime/time/mod.rs
+++ b/tokio/src/runtime/time/mod.rs
@@ -198,11 +198,11 @@ impl Driver {
 
         // Finds out the min expiration time to park.
         let expiration_time = {
-            let mut wheels_lock = rt_handle.time().inner.wheels.write();
+            let wheels_lock = rt_handle.time().inner.wheels.read();
             let expiration_time = wheels_lock
                 .0
-                .iter_mut()
-                .filter_map(|wheel| wheel.get_mut().next_expiration_time())
+                .iter()
+                .filter_map(|wheel| wheel.lock().next_expiration_time())
                 .min();
 
             rt_handle

Probably there's a better real fix that someone with more brains can do to redesign this with less mutex contention.

[Darksonn]

The suggested change is incorrect. The code was like that previously in Tokio v1.38.0 which caused a serious bug, and we had to make a bugfix release v1.38.1 to fix it.

[Darksonn]

It looks like I asked for a test, but we never actually added one to get the bugfix out more quickly. We should probably add a test that would catch this.