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: optimize header list size calculations #750

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

Noah-Kennedy
Copy link
Contributor

This speeds up loading blocks in cases where we have many headers already.

@Noah-Kennedy Noah-Kennedy force-pushed the noah/speed-up-decoding branch from be897a2 to d5b89a2 Compare February 21, 2024 23:14
@Noah-Kennedy
Copy link
Contributor Author

Accidentally targeted it at 0.3, though I think this is enough of a perf improvement to justify a backport.

We can just forwardport this to 0.4 after I guess?

@Noah-Kennedy Noah-Kennedy force-pushed the noah/speed-up-decoding branch 2 times, most recently from dff5782 to eb64392 Compare February 21, 2024 23:21
This speeds up loading blocks in cases where we have many headers already.
@Noah-Kennedy Noah-Kennedy force-pushed the noah/speed-up-decoding branch from eb64392 to 0e7ca68 Compare February 21, 2024 23:26
@Noah-Kennedy Noah-Kennedy changed the title headers: optimize header list size calculations per: optimize header list size calculations Feb 21, 2024
@Noah-Kennedy Noah-Kennedy changed the title per: optimize header list size calculations perf: optimize header list size calculations Feb 21, 2024
@@ -892,6 +901,8 @@ impl HeaderBlock {

headers_size += decoded_header_size(name.as_str().len(), value.len());
if headers_size < max_header_list_size {
self.field_size +=
decoded_header_size(name.as_str().len(), value.len());

Choose a reason for hiding this comment

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

Should we check that decoded_header_size won't exceed max_header_list_sized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is covered under other checks elsewhere

Choose a reason for hiding this comment

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

Nvm, that's exactly what this does

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I don't know why I said that lol, it's literally checked right here.

@seanmonstar seanmonstar merged commit 94e80b1 into hyperium:0.3.x Feb 22, 2024
6 checks passed
@Noah-Kennedy Noah-Kennedy deleted the noah/speed-up-decoding branch February 22, 2024 01:56
seanmonstar pushed a commit that referenced this pull request Feb 22, 2024
This speeds up loading blocks in cases where we have many headers already.
seanmonstar pushed a commit that referenced this pull request Feb 22, 2024
This speeds up loading blocks in cases where we have many headers already.
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