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

HTTP background thread consumes 100% CPU #107

Closed
svyatonik opened this issue Jun 3, 2020 · 3 comments · Fixed by #109 or #150
Closed

HTTP background thread consumes 100% CPU #107

svyatonik opened this issue Jun 3, 2020 · 3 comments · Fixed by #109 or #150

Comments

@svyatonik
Copy link
Contributor

The reason is that FuturesUnordered::poll_next() always returns Poll::Ready(None) if it is empty (at the start), or when all its futures have completed. So we instantly wake up here and proceed to the next iteration.

Bonus issue: it seems that completed futures are never removed from pending_requests.

@tomaka
Copy link
Contributor

tomaka commented Jun 9, 2020

I haven't tested this, but it seems that something like that is much more elegant than #108

diff --git a/src/transport/http/client.rs b/src/transport/http/client.rs
index 96c854a..fb864e4 100644
--- a/src/transport/http/client.rs
+++ b/src/transport/http/client.rs
@@ -222,7 +222,19 @@ fn background_thread(mut requests_rx: mpsc::Receiver<FrontToBack>) {
         let mut pending_requests = stream::FuturesUnordered::new();
 
         loop {
-            let rq = match future::select(requests_rx.next(), pending_requests.next()).await {
+            let event = {
+                let next_pending_finished = async {
+                    if !pending_requests.is_empty() {
+                        pending_requests.next()
+                    } else {
+                        futures::pending!()
+                    }
+                };
+
+                future::select(requests_rx.next(), next_pending_finished).await
+            };
+
+            let rq = match event {
                 // We received a request from the foreground.
                 future::Either::Left((Some(rq), _)) => rq,
                 // The channel with the foreground has closed.

@svyatonik
Copy link
Contributor Author

@tomaka You're right - thanks :) I've missed the fact that futures are actually removed from the UnorderedFutures when they are completed (so my 'bonus issue' looks invalid now :) ). Will prepare another PR, based on your solution.

@tomaka
Copy link
Contributor

tomaka commented Jun 9, 2020

Again I haven't tested, but I think something like that should be quite elegant:

let event = loop {
    if !pending_requests.is_empty() {
        if let future::Either::Left(ev) = future::select(..., ...).await { break ev }; 
    } else {
        break requests_rx.next().await
    }
};

match event {
    ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants