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

Revert "fix(node/http): do not buffer first chunk (#2989)" #3013

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

bartlomieju
Copy link
Member

This reverts commit eca4dbf.

This commit caused performance regression in express server from
25k rps to 20k rps.

CC @kt3k

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM

Can you show how to reproduce this bug so we can re-introduce the fix, but without the regression?

@kt3k
Copy link
Member

kt3k commented Dec 17, 2022

The below server responds immediately with chunk "A", and then "B" 1 sec later with the fix, but it responds "AB" after 1 sec without the fix.

import * as http from "./node/http.ts";
http.createServer((_, res) => {
  res.write("A");
  setTimeout(() => {
    res.write("B");
    res.end();
  }, 1000)
}).listen(3000);

@bartlomieju bartlomieju merged commit ebd670c into denoland:main Dec 19, 2022
@bartlomieju bartlomieju deleted the revert_2989 branch December 19, 2022 12:58
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.

None yet

3 participants