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

use a ring buffer for the frame queue in the framer #3826

Closed
marten-seemann opened this issue May 13, 2023 · 1 comment · Fixed by #3857
Closed

use a ring buffer for the frame queue in the framer #3826

marten-seemann opened this issue May 13, 2023 · 1 comment · Fixed by #3857

Comments

@marten-seemann
Copy link
Member

The framer uses a slice of stream IDs, containing an (ordered) list of the streams that have data to send:

streamQueue []protocol.StreamID

Using a slice here is problematic, as for every packet we send (assuming it contains one STREAM frame), we consume and append one element to the slice. It therefore leads to regular allocations in the data send path.

This allocation is responsible for more than 10% of the runtime of AppendStreamFrames:
image

Instead, we should use a ring buffer. It can start with a fairly small capacity (4 maybe?) and double that capacity once this is not enough. In this case, the maximum capacity is limited by the number of concurrent streams, so the ring buffer would not have to deal with this.

I imagine that it would make sense to implement a generic ring buffer and use that one in the framer.

@Glonee
Copy link
Contributor

Glonee commented May 23, 2023

https://github.com/golang/go/blob/e4e8f9b8ffff9d1bcbaaf4b98307d0b88c26678f/src/net/http/transport.go#L1264-L1303
Here is a efficient queue implemention in the standard library. It is similar to ringbuffer but the implemention is much simpler. Maybe we can implement a generic version and use it?

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

Successfully merging a pull request may close this issue.

2 participants