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 issue with Balsa Parser #29203

Closed
zhxie opened this issue Aug 23, 2023 · 4 comments
Closed

Performance issue with Balsa Parser #29203

zhxie opened this issue Aug 23, 2023 · 4 comments
Labels
area/http quiche stale stalebot believes this issue/PR has not been touched recently

Comments

@zhxie
Copy link
Contributor

zhxie commented Aug 23, 2023

Title: Performance issue with Balsa Parser

Description:

Since we are going to migrate from http-parser to Balsa Parser (#21245, #28896), I did some quick benchmarking recently and found that Balsa Parser is 10% slower than the current http-parser implementation, though it claims to have similar performance.

# http-parser
All done 5496764 calls (plus 64 warmup) 3.493 ms avg, 18322.5 qps
# Balsa Parser
All done 5058443 calls (plus 64 warmup) 3.795 ms avg, 16861.4 qps

The benchmarking is performed with Envoy running in a pinned single thread, echoing back 1kB headers with Fortio from 64 concurrent connections, no filter except HCM and HTTP router, full CPU load.

I also made a draft commit with llhttp implementatoin, a successor of http-parser, which has been mentioned a lot of times, e.g., #5155, #9033 and #15814. The result looks good.

# http-parser
All done 5496764 calls (plus 64 warmup) 3.493 ms avg, 18322.5 qps
# llhttp
All done 5626763 calls (plus 64 warmup) 3.412 ms avg, 18755.8 qps

Note that I did not exactly follow the What are best practices for benchmarking Envoy? article when collecting this benchmarking data.

I know that enable Balsa Parser will let us support some requested features, do you think we can add an API option to support different parsers instead of using runtime guard to introduce a new one and deprecate the old one?

@zhxie zhxie added the triage Issue requires triage label Aug 23, 2023
@RyanTheOptimist RyanTheOptimist added area/http quiche and removed triage Issue requires triage labels Aug 25, 2023
@RyanTheOptimist
Copy link
Contributor

cc: @bencebeky @birenroy

@RyanTheOptimist
Copy link
Contributor

cc: @diannahu

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Sep 25, 2023
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/http quiche stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

No branches or pull requests

2 participants