okhttp: avoid unnecessary Header allocation for lowercase names - #3
Open
skrcode wants to merge 2 commits into
Open
Conversation
Reuse the immutable input Header when its name is already canonical, while preserving allocation and case folding for mixed-case names. Add a regression test for repeated mixed-case indexing. Generated-by: JAIPilot Cloud Signed-off-by: Suraj Rajan <5915832+skrcode@users.noreply.github.com>
Make the allocation-saving branch a durable, deterministic regression property by asserting that the dynamic table retains the immutable input Header. Generated-by: JAIPilot Cloud Signed-off-by: Suraj Rajan <5915832+skrcode@users.noreply.github.com>
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
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.
This is an optional, independently reviewed follow-up to grpc#12907. It does not open a duplicate PR against upstream.
What
ByteString.toAsciiLowercase()leaves an already-lowercase header name unchanged. For that common gRPC path—including:pathand:authority—the writer can reuse the immutable inputHeaderin the dynamic table instead of allocating an equivalent object. Mixed-case input still receives a canonical lowercaseHeader; focused tests lock both behaviors.Evidence
Headerobjects perwriteHeaderscall.lowercaseHeaderIsReusedInDynamicTabletest deterministically asserts the allocation-saving branch by checking that the writer stores the same immutableHeaderinstance.:grpc-okhttp:buildpassed, 359 tests, 0 failures/errors (3 skipped).d0272ef78f4e78983cb1f19ac25d1561ec26cc1b.The original bot draft is skrcode#2. Independent review replaced its intentional reference comparison with value equality because the reference form introduced an Error Prone warning; this version preserves the zero-allocation common path without adding that warning.
Scope and limitations
Two existing files, no public API or wire-format change. The ThreadMXBean harness was intentionally not committed, so the durable proof is the identity assertion, mixed-case regression, and identical repository-native clean builds. No wall-clock speed claim is made.
Both commits include a
Generated-by: JAIPilot Cloudtrailer for transparency.