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

Avoid overhead of constructing debug messages that are not printed #54

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

hlinnaka
Copy link
Contributor

I noticed that the neon pageserver unit tests were taking much longer with tokio-epoll-uring than with std:fs. It was because many of the tests used tokio::time:pause(), and then did a call to tokio::time::timeout() with a long 1h timeout. With tokio-epoll-uring, the loop in poller_impl_impl() woke up 36000 times, i.e. once for every 100 ms of virtual time, before the timeout was reached. The significance of that varies greatly depend on debug vs release build; it's particularly bad when I use nightly compiler with cranelift codegen backend.

It would be better to avoid the wakeups every 100 ms altogether, if the debug-level events are not enabled, but it scares me to have such different behavior depending on a debug-level. It sounds like a recipe for very hard-to-debug bugs. Or perhaps the debug dumping could be just removed altogether? That would be good for saving battery life when the system is idle.

@hlinnaka
Copy link
Contributor Author

Not suitable for including as a test, but this can be used to demo the effect:

diff --git a/tokio-epoll-uring/Cargo.toml b/tokio-epoll-uring/Cargo.toml
index 07e12a8..7b3f9fa 100644
--- a/tokio-epoll-uring/Cargo.toml
+++ b/tokio-epoll-uring/Cargo.toml
@@ -22,3 +22,4 @@ tempfile = "3.6.0"
 tracing-subscriber = "*"
 os_pipe = "1.1.4"
 assert-panic = "1.0.1"
+tokio = { version = "1.29.1", features = [ "test-util" ] }
diff --git a/tokio-epoll-uring/src/system/tests.rs b/tokio-epoll-uring/src/system/tests.rs
index 0168ef1..f8e994a 100644
--- a/tokio-epoll-uring/src/system/tests.rs
+++ b/tokio-epoll-uring/src/system/tests.rs
@@ -298,3 +298,12 @@ async fn test_write() {
 
     drop(fd);
 }
+
+
+#[tokio::test]
+async fn test_nothing() {
+    let _system = System::launch().await.unwrap();
+
+    tokio::time::pause();
+    let _ = tokio::time::timeout(Duration::from_secs(360000), std::future::pending::<()>()).await;
+}

The test finishes much quicker with this PR.

@hlinnaka hlinnaka requested review from a team and problame and removed request for a team September 30, 2024 17:44
I noticed that the neon pageserver unit tests were taking much longer
with tokio-epoll-uring than with std:fs. It was because many of the
tests used tokio::time:pause(), and then did a call to
tokio::time::timeout() with a long 1h timeout. With tokio-epoll-uring,
the loop in poller_impl_impl() woke up 36000 times, i.e. once for
every 100 ms of virtual time, before the timeout was reached. The
significance of that varies greatly depend on debug vs release build;
it's particularly bad when I use nightly compiler with cranelift
codegen backend.

It would be better to avoid the wakeups every 100 ms altogether, if
the debug-level events are not enabled, but it scares me to have such
different behavior depending on a debug-level. It sounds like a recipe
for very hard-to-debug bugs. Or perhaps the debug dumping could be
just removed altogether? That would be good for saving battery life
when the system is idle.
@hlinnaka hlinnaka force-pushed the poller-timeout-cheaper branch from d7b9ee9 to 833c4d1 Compare October 1, 2024 10:44
@problame problame merged commit 2256845 into main Oct 28, 2024
6 checks passed
@problame problame deleted the poller-timeout-cheaper branch October 28, 2024 19:41
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

Successfully merging this pull request may close these issues.

2 participants