Skip to content

Commit 397d3e7

Browse files
kenkangxgweejona86
authored andcommitted
okhttp: Optimize HPACK to index :path
Allow the :path pseudo-header to be added to the HPACK dynamic table in the gRPC-Java OkHttp transport. In gRPC, the :path header typically represents the service and method name (e.g., /package.Service/Method), which is constant across multiple calls to the same method. The previous HPACK implementation, inherited from OkHttp, did not index :path, treating it as potentially highly variable. By allowing :path to be indexed, subsequent gRPC calls to the same method can benefit from HPACK's dynamic table, reducing header size.
1 parent f021bef commit 397d3e7

2 files changed

Lines changed: 35 additions & 11 deletions

File tree

  • okhttp/third_party/okhttp

okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Hpack.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -490,9 +490,14 @@ void writeHeaders(List<io.grpc.okhttp.internal.framed.Header> headerBlock) throw
490490
writeByteString(name);
491491
writeByteString(value);
492492
insertIntoDynamicTable(header);
493-
} else if (name.startsWith(PSEUDO_PREFIX) && !io.grpc.okhttp.internal.framed.Header.TARGET_AUTHORITY.equals(name)) {
494-
// Follow Chromes lead - only include the :authority pseudo header, but exclude all other
495-
// pseudo headers. Literal Header Field without Indexing - Indexed Name.
493+
} else if (name.startsWith(PSEUDO_PREFIX)
494+
&& !io.grpc.okhttp.internal.framed.Header.TARGET_AUTHORITY.equals(name)
495+
&& !io.grpc.okhttp.internal.framed.Header.TARGET_PATH.equals(name)) {
496+
// Allow :authority and :path pseudo headers to be indexed. Other pseudo headers are not
497+
// indexed.
498+
// This is a departure from the original Chrome-inspired behavior, as gRPC paths
499+
// (ServiceName/MethodName)
500+
// are stable and benefit from indexing.
496501
writeInt(headerNameIndex, PREFIX_4_BITS, 0);
497502
writeByteString(value);
498503
} else {

okhttp/third_party/okhttp/test/java/io/grpc/okhttp/internal/framed/HpackTest.java

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -272,14 +272,18 @@ public void readerEviction() throws IOException {
272272

273273
/**
274274
* http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-12#appendix-C.2.2
275+
*
276+
* <p>This test mimics the draft example which uses ":path", but since gRPC-Java now indexes
277+
* ":path" for performance, we use ":method" (with a non-static value like "PUT") to verify the
278+
* "Literal Header Field without Indexing - Indexed Name" representation.
275279
*/
276280
@Test public void literalHeaderFieldWithoutIndexingIndexedName() throws IOException {
277-
List<Header> headerBlock = headerEntries(":path", "/sample/path");
281+
List<Header> headerBlock = headerEntries(":method", "PUT");
278282

279-
bytesIn.writeByte(0x04); // == Literal not indexed ==
280-
// Indexed name (idx = 4) -> :path
281-
bytesIn.writeByte(0x0c); // Literal value (len = 12)
282-
bytesIn.writeUtf8("/sample/path");
283+
bytesIn.writeByte(0x02); // == Literal not indexed ==
284+
// Indexed name (idx = 2) -> :method
285+
bytesIn.writeByte(0x03); // Literal value (len = 3)
286+
bytesIn.writeUtf8("PUT");
283287

284288
hpackWriter.writeHeaders(headerBlock);
285289
assertEquals(bytesIn, bytesOut);
@@ -1104,14 +1108,29 @@ public void dynamicTableIndexedHeader() throws IOException {
11041108
}
11051109

11061110
@Test
1107-
public void doNotIndexPseudoHeaders() throws IOException {
1111+
public void pseudoHeaderIndexing() throws IOException {
1112+
// :method is not indexed (unless it's GET or POST, which are in the static table)
11081113
hpackWriter.writeHeaders(headerEntries(":method", "PUT"));
11091114
assertBytes(0x02, 3, 'P', 'U', 'T');
11101115
assertEquals(0, hpackWriter.dynamicTableHeaderCount);
11111116

1117+
// :path should now be indexed
11121118
hpackWriter.writeHeaders(headerEntries(":path", "/okhttp"));
1113-
assertBytes(0x04, 7, '/', 'o', 'k', 'h', 't', 't', 'p');
1114-
assertEquals(0, hpackWriter.dynamicTableHeaderCount);
1119+
assertBytes(0x44, 7, '/', 'o', 'k', 'h', 't', 't', 'p');
1120+
assertEquals(1, hpackWriter.dynamicTableHeaderCount);
1121+
// Second time should be an index
1122+
hpackWriter.writeHeaders(headerEntries(":path", "/okhttp"));
1123+
assertBytes(0xbe);
1124+
assertEquals(1, hpackWriter.dynamicTableHeaderCount);
1125+
1126+
// :authority should be indexed
1127+
hpackWriter.writeHeaders(headerEntries(":authority", "test.com"));
1128+
assertBytes(0x41, 8, 't', 'e', 's', 't', '.', 'c', 'o', 'm');
1129+
assertEquals(2, hpackWriter.dynamicTableHeaderCount);
1130+
// Second time should be an index
1131+
hpackWriter.writeHeaders(headerEntries(":authority", "test.com"));
1132+
assertBytes(0xbe);
1133+
assertEquals(2, hpackWriter.dynamicTableHeaderCount);
11151134
}
11161135

11171136
@Test

0 commit comments

Comments
 (0)