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

Reserve capacity when decoding HPACK headers #118

Merged
merged 1 commit into from
May 16, 2019

Conversation

glbrntt
Copy link
Contributor

@glbrntt glbrntt commented May 16, 2019

Motivation:

I was seeing frequent reallocations of the headers array as it
resized during decoding.

Modifications:

Reserve capacity for at least 16 HPACK headers during decoding.

Result:

Fewer reallocations at the cost of allocating more than we might need.

Motivation:

I was seeing frequent reallocations of the headers array as it
resized during decoding.

Modifications:

Reserve capacity for at least 16 HPACK headers during decoding.

Result:

Fewer reallocations at the cost of allocating more than we might need.
@Lukasa Lukasa requested a review from weissi May 16, 2019 09:50
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label May 16, 2019
@Lukasa Lukasa added this to the 1.2.1 milestone May 16, 2019
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

LGTM, seems like a nice change.

@weissi
Copy link
Member

weissi commented May 16, 2019

great job @glbrntt

from

DEBUG: [["remaining_allocations": 0, "total_allocations": 14000], ["remaining_allocations": 0, "total_allocations": 14000], ["remaining_allocations": 0, "total_allocations": 14000], ["remaining_allocations": 0, "total_allocations": 14000], ["total_allocations": 14000, "remaining_allocations": 0], ["remaining_allocations": 0, "total_allocations": 14000], ["total_allocations": 14000, "remaining_allocations": 0], ["total_allocations": 14000, "remaining_allocations": 0], ["total_allocations": 14000, "remaining_allocations": 0], ["total_allocations": 14000, "remaining_allocations": 0]]

to

DEBUG: [["total_allocations": 5000, "remaining_allocations": 0], ["total_allocations": 5000, "remaining_allocations": 0], ["total_allocations": 5000, "remaining_allocations": 0], ["remaining_allocations": 0, "total_allocations": 5000], ["total_allocations": 5000, "remaining_allocations": 0], ["total_allocations": 5000, "remaining_allocations": 0], ["total_allocations": 5000, "remaining_allocations": 0], ["total_allocations": 5000, "remaining_allocations": 0], ["remaining_allocations": 0, "total_allocations": 5000], ["total_allocations": 5000, "remaining_allocations": 0]]

so this goes from 14 allocations to 5 for 3 example HPACK decodes.

Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

looks good to me, thanks George!

@weissi weissi merged commit d66cd24 into apple:master May 16, 2019
@glbrntt glbrntt deleted the hpack-decode-reserve-capacity branch September 24, 2019 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants