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

Performance opportunities #1690

Closed
larseggert opened this issue Feb 27, 2024 · 5 comments
Closed

Performance opportunities #1690

larseggert opened this issue Feb 27, 2024 · 5 comments

Comments

@larseggert
Copy link
Collaborator

diff --git a/neqo-common/src/udp.rs b/neqo-common/src/udp.rs
index 7ad0b976..ded473cb 100644
--- a/neqo-common/src/udp.rs
+++ b/neqo-common/src/udp.rs
@@ -71,6 +71,8 @@ impl Socket {

     /// Receive a UDP datagram on the specified socket.
     pub fn recv(&self, local_address: &SocketAddr) -> Result<Option<Datagram>, io::Error> {
+        // TODO: The next line uses about 7% of CPU time on neqo-client to zero out the buffer.
+        // Figure out a way to avoid this.
         let mut buf = [0; u16::MAX as usize];

         let mut meta = RecvMeta::default();
diff --git a/neqo-transport/src/recv_stream.rs b/neqo-transport/src/recv_stream.rs
index 5da80d60..518ee45d 100644
--- a/neqo-transport/src/recv_stream.rs
+++ b/neqo-transport/src/recv_stream.rs
@@ -364,6 +364,8 @@ impl RxStreamOrderer {
                 keep = true;
             }
             if keep {
+                // TODO: We're spending about 4% of client CPU time in `split_off`, and almost 8%
+                // in this `read` function. Optimize.
                 let mut keep = self.data_ranges.split_off(&range_start);
                 mem::swap(&mut self.data_ranges, &mut keep);
                 return copied;
@martinthomson
Copy link
Member

It seems to me like we could try using a Vec or VecDeque for the receive buffer. That would mean copying bytes into a larger buffer (with wrap-around), but it might make these trim operations easier to manage.

A simpler approach would be to try BTreeMap::retain as an alternative.

@larseggert
Copy link
Collaborator Author

For the first one, we probably need to do something like https://github.com/quinn-rs/quinn/blob/main/quinn/src/endpoint.rs#L397, but my Rust-fu isn't up to it.

@mxinden
Copy link
Collaborator

mxinden commented Feb 28, 2024

I can tackle the first one as part of the prototype proposed in #1693.

@larseggert
Copy link
Collaborator Author

Note that #1751 really changed where cycles are spent. It's not clear whether these are still bottlenecks.

@larseggert
Copy link
Collaborator Author

Hence closing.

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

No branches or pull requests

3 participants