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

perf(ext/http): increase buffer size to 64kb #16077

Closed
wants to merge 1 commit into from

Conversation

marcosc90
Copy link
Contributor

@marcosc90 marcosc90 commented Sep 28, 2022

Towards #13608

The current buffer size is the bottleneck. This PR increases the buffer size to 64kb, even if the largest possible size for a TLS packet is 16 * 1024 + 256, we're reading at a slower rate than the upload speed, so packets are being buffered,

let len = min(buf.len(), chunk.len());

You can check that chunk.len() is usually greater than the max TLS packet size.

The higher the buffer size the faster it'll be, 64kb seems like a reasonable default value.

On my computer, streaming a 2gb upload to disk, takes 2.4s with this update down from 4.5s.


Adding a bufSize/highWaterMark option to Deno.serveHttp or httpConn.nextRequest() to change the default value would be a good next step.

Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@marcosc90
Copy link
Contributor Author

Since we started working towards FastStreams this code path no longer exists. Change implemented in #16096

@marcosc90 marcosc90 closed this Sep 30, 2022
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