Add option to disable HPACK dynamic table - #12973
Open
petedmarsh wants to merge 2 commits into
Open
Conversation
Add client and server builder controls that disable HPACK dynamic table use in both directions. Advertise a zero header table size and keep the encoder table pinned at zero when the peer changes its setting. Add encoder, handler, builder, transport, and interoperability coverage. AI assistance: OpenAI Codex (GPT-5) was used to review these HPACK changes and strengthen the tests.
Apply SETTINGS_HEADER_TABLE_SIZE to the outbound HPACK writer before acknowledging it, so the next header block emits the required dynamic table size update. Stop applying the peer encoder setting to the inbound decoder. Add framed unit coverage and bidirectional OkHttp-Netty regression tests. The tests verify that repeated calls remain on one transport. AI assistance: OpenAI Codex (GPT-5) was used to implement and test this grpc-okhttp compatibility fix.
petedmarsh
changed the title
Member
|
Can you describe what your goal is? It seems like it is to reduce idle memory usage? The okhttp bug has been on our radar, but needed some further investigation (#12819 . #12818 is similar but I saw it didn't remove hpackReader.headerTableSizeSetting(), so it was highly suspect; it was also AI-generated, and we really want a human to look at it). We will definitely want to keep any changes there as a separate commit. |
Author
|
Where I work we send a lot of high cardinality metadata/header values on
our internal request paths and we have seen via profiling that the dynamic
HPACK table just burns CPU and doesn't save network bytes.
I'm perfectly happy if this PR is not merged BUT I would really like the
ability to turn off the dynamic HPACK table one way or another.
…On Thu, 6 Aug 2026, 19:57 Eric Anderson, ***@***.***> wrote:
*ejona86* left a comment (grpc/grpc-java#12973)
<#12973 (comment)>
Can you describe what your goal is? It seems like it is to reduce idle
memory usage?
The okhttp bug has been on our radar, but needed some further
investigation (#12819 <#12819> .
#12818 <#12818> is similar but I
saw it didn't remove hpackReader.headerTableSizeSetting(), so it was highly
suspect; it was also AI-generated, and we really want a human to look at
it). We will definitely want to keep any changes there as a separate commit.
—
Reply to this email directly, view it on GitHub
<#12973?email_source=notifications&email_token=AAJDC6UHQTM3WGO2JIQ3OVT5ITBI7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMRQHAYDAOJVGEY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5208009511>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJDC6XX6ZA72ZA3U5WQT5L5ITBI7AVCNFSNUABEKJSXA33TNF2G64TZHMZDONZSHE4TENR3JFZXG5LFHM2TAOBSGYYDEMZRGSQXMAQ>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AAJDC6XA2JZI6COD62Q7WOL5ITBI7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMRQHAYDAOJVGEY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/AAJDC6T2T545PBQNIAGMKHD5ITBI7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMRQHAYDAOJVGEY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Member
|
Can you share what function(s) is hot? Is it the key/value hashing? Receiving indexed values in HPACK seems reasonably cheap. I expect it is just the encoder costing you. One thing for us to consider is this approach looks to be incompatible with Http2FrameCodecBuilder. We really need to migrate off the old Netty API that we are using, at which point we'll be limited to what Http2FrameCodecBuilder offers. (So then we have to either drop this feature or upstream it to Netty.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
disableHpackDynamicTable()to the Netty client and server builders.When enabled, Netty uses a zero-sized HPACK dynamic table and advertises
SETTINGS_HEADER_TABLE_SIZE = 0to its peer. The default remains unchanged.Also fix grpc-okhttp to apply a peer's header-table setting to its outbound encoder instead of its inbound decoder.
Compliant peers require no configuration changes. Older grpc-okhttp releases must be upgraded before enabling this option on their Netty peer.
OpenAI Codex (GPT-5) was used to implement all changes in this PR.