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

[http] Simplify H1 codec by removing the reserve/commit pattern #9825

Merged
merged 2 commits into from
Jan 28, 2020

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Jan 24, 2020

This simplifies the H/1 codec by removing the pre-buffer reserved_iovec_ in the HTTP1 codec.

This change reduces effective memory usage by removing the need to round up when reserving memory blocks in OwnedImpl::reserve. The output_buffer_ in the codec is a watermark buffer that on adds will checkHighWatermark. Performance may suffer slightly due to an increase in number of calls to checkHighWatermark while serializing headers.

(Before the change, watermarks were checked at most twice in ConnectionImpl::reserveBuffer (commit if needed and reserve), and not in the following calls to add/copy to the buffer. Now, add/copyToBuffer directly call the watermark buffer, so check watermarks are increased.)

Risk level: medium/high
Testing: existing tests pass (suggestions welcome for more if I can peak at the connection's buffer)

Signed-off-by: Asra Ali [email protected]

Signed-off-by: Asra Ali <[email protected]>
@mattklein123
Copy link
Member

Looks like it needs a format fix and then will take a look, thank you!

/wait

Signed-off-by: Asra Ali <[email protected]>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

LGTM. Will defer to @alyssawilk for final approval. Thank you!

@mattklein123 mattklein123 merged commit caad934 into envoyproxy:master Jan 28, 2020
alyssawilk pushed a commit that referenced this pull request Jan 12, 2022
…ders (#19115)

Commit Message: minor perf: reduce fine grained buffer access when encoding HTTP1 headers

Additional Description:
The HTTP1 codec performs a large number of buffer APIs calls during the encoding of HTTP headers, which introduces an additional CPU overhead (3-4%). This PR implements a buffer helper as a cache to reduce direct buffer writes to improving the overall performance.
Check this #19103 (comment) for some more related info.
This PR reverts #9825. At the same time, I also try my best to make the code simpler and easier to maintain.

Risk Level: High.
Testing: Waiting.
Docs Changes: N/A.
Release Notes: N/A.

Signed-off-by: wbpcode <[email protected]>
joshperry pushed a commit to joshperry/envoy that referenced this pull request Feb 13, 2022
…ders (envoyproxy#19115)

Commit Message: minor perf: reduce fine grained buffer access when encoding HTTP1 headers

Additional Description:
The HTTP1 codec performs a large number of buffer APIs calls during the encoding of HTTP headers, which introduces an additional CPU overhead (3-4%). This PR implements a buffer helper as a cache to reduce direct buffer writes to improving the overall performance.
Check this envoyproxy#19103 (comment) for some more related info.
This PR reverts envoyproxy#9825. At the same time, I also try my best to make the code simpler and easier to maintain.

Risk Level: High.
Testing: Waiting.
Docs Changes: N/A.
Release Notes: N/A.

Signed-off-by: wbpcode <[email protected]>
Signed-off-by: Josh Perry <[email protected]>
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.

3 participants