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

IO flushed twice if using JSON.build or YAML.build #10709

Closed
matthewmcgarvey opened this issue May 13, 2021 · 2 comments · Fixed by #10716
Closed

IO flushed twice if using JSON.build or YAML.build #10709

matthewmcgarvey opened this issue May 13, 2021 · 2 comments · Fixed by #10716
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:networking

Comments

@matthewmcgarvey
Copy link
Contributor

JSON

  1. @io.flush
  2. io.flush

YAML

  1. @io.flush
  2. io.flush

Trying to figure out why calling {hello: "world"}.to_json(http_response) is so much slower than http_response.print({hello: "world"}.to_json). This seems like it could be one of the issues since my guess is that the response io would be flushed 3 times per request in the slow version and only once in the quicker. PR updating TechEmpower benchmarks: TechEmpower/FrameworkBenchmarks#6584

I tried referencing other builders but it seems that every one does it differently:

@matthewmcgarvey matthewmcgarvey added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label May 13, 2021
@matthewmcgarvey
Copy link
Contributor Author

Looks like the double flushes were added in this PR #9321 which was made to align the builders flush methods though it didn't remove the old flush calls like it probably should have.

@straight-shoota
Copy link
Member

Yeah, I suppose I missed in #9321 that the duplicate flushes should be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:networking
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants