Skip to content

okhttp: avoid unnecessary Header allocation for lowercase names - #3

Open
skrcode wants to merge 2 commits into
AgraVator:fix-okhttp-hpack-eviction-12819from
skrcode:offer/grpc-12907-hpack-allocation
Open

okhttp: avoid unnecessary Header allocation for lowercase names#3
skrcode wants to merge 2 commits into
AgraVator:fix-okhttp-hpack-eviction-12819from
skrcode:offer/grpc-12907-hpack-allocation

Conversation

@skrcode

@skrcode skrcode commented Aug 22, 2026

Copy link
Copy Markdown

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 :path and :authority—the writer can reuse the immutable input Header in the dynamic table instead of allocating an equivalent object. Mixed-case input still receives a canonical lowercase Header; focused tests lock both behaviors.

Evidence

  • JAIPilot instrumented a representative lowercase gRPC header block at 5 → 0 new Header objects per writeHeaders call.
  • Its five-run ThreadMXBean harness reported 8,488 → 8,368 allocated bytes/iteration over 200,000 iterations per side. This is allocation evidence, not a wall-clock claim.
  • The committed lowercaseHeaderIsReusedInDynamicTable test deterministically asserts the allocation-saving branch by checking that the writer stores the same immutable Header instance.
  • Independent Corretto 21 replay of the exact source head: clean :grpc-okhttp:build passed, 359 tests, 0 failures/errors (3 skipped).
  • Independent replay of this exact candidate: the same 50-task clean build passed, 361 tests, 0 failures/errors (3 skipped).
  • The branch starts directly at exact source head 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 Cloud trailer for transparency.

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>
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.

1 participant