Skip to content

perf: reduce HashMap/collection allocation overhead in gateway path#48662

Open
xinlian12 wants to merge 23 commits intoAzure:mainfrom
xinlian12:perf/hashmap-collection-allocation
Open

perf: reduce HashMap/collection allocation overhead in gateway path#48662
xinlian12 wants to merge 23 commits intoAzure:mainfrom
xinlian12:perf/hashmap-collection-allocation

Conversation

@xinlian12
Copy link
Copy Markdown
Member

@xinlian12 xinlian12 commented Apr 1, 2026

Performance: Reduce HashMap/Collection Allocation Overhead in Gateway Path

Motivation

JFR profiling of the baseline (main) under high-concurrency gateway workloads revealed that HashMap-related allocations (HashMap$Node, HashMap, HashMap$ValueIterator) and HTTP header collections (DefaultHeaders$HeaderEntry, HttpHeader) are responsible for a significant share of total object allocation churn.

Baseline JFR allocation profile (c128 Read HTTP/1, ObjectAllocationSample, 10-min recording):

Class % of Total Allocation
HashMap$Node 6.9%
DefaultHeaders$HeaderEntry 6.8%
HashMap$ValueIterator 1.3%
HttpHeader 0.9%
HashMap 0.7%
HttpHeaders 0.6%
HashMap$Node[] 0.5%
Total targeted ~10.9%

Root causes:

  1. HashMap<>() default initial capacity (16) forces 1-2 resize+rehash cycles for typical gateway responses with 20-30 headers, creating throwaway HashMap$Node[] arrays and re-hashed HashMap$Node entries
  2. StoreResponse constructor converts HttpHeaders to Map via HttpUtils.asMap() on every response, allocating a throwaway HashMap$ValueIterator and rebuilding all HashMap$Node entries
  3. HttpHeaders in RxGatewayStoreModel.getHttpRequestHeaders() is undersized, causing internal HashMap resize
  4. Redundant toLowerCase() calls on header keys that are already normalized

Changes

  1. Right-sized HashMap initial capacity: HashMap<>(32) instead of HashMap<>() in RxDocumentServiceRequest, and mapCapacityForSize() helper in HttpUtils to avoid rehashing
  2. Eliminate HashMap to HttpHeaders to HashMap round-trip: StoreResponse now accepts HttpHeaders directly, removing intermediate asMap() conversion that created throwaway HashMap$ValueIterator and HashMap$Node arrays
  3. Pre-sized HttpHeaders in RxGatewayStoreModel: sized to defaultHeaders.size() + headers.size() to avoid internal HashMap resize
  4. Remove redundant toLowerCase() calls: HttpHeaders.set() already normalizes keys; callers no longer double-normalize creating extra String objects

Benchmark Results

Test matrix: 1-tenant {c1, c8, c32, c128} {HTTP/1, HTTP/2} + 30-tenant c3 {HTTP/1, HTTP/2}, GATEWAY mode, ReadThroughput, 10 min per round, 3 independent JVM rounds per config. All endpoints pinned to the same Cosmos gateway frontend via /etc/hosts to eliminate Azure Traffic Manager routing variance. Both main and hashmap-alloc tested on the same VM per config.

1-Tenant Throughput & Latency

Config Concurrency Protocol main (ops/s) PR (ops/s) Δ ops main p99 (ms) PR p99 (ms) main mean (ms) PR mean (ms)
Read c1 HTTP/1 444 445 +0.4% 5.57 5.47 2.24 2.26
Read c1 HTTP/2 450 448 -0.4% 5.42 5.45 2.21 2.23
Read c8 HTTP/1 3,714 3,711 -0.1% 5.13 5.04 2.13 2.12
Read c8 HTTP/2 3,702 3,704 +0.0% 5.15 5.13 2.13 2.12
Read c32 HTTP/1 15,137 15,190 +0.4% 4.70 4.70 2.07 2.06
Read c32 HTTP/2 15,537 15,525 -0.1% 4.70 4.70 2.03 2.04
Read c128 HTTP/1 50,220 50,519 +0.6% 5.39 5.39 2.44 2.43
Read c128 HTTP/2 50,533 50,553 +0.0% 5.38 5.40 2.44 2.44

Note: p99 is the cumulative operation latency (SDK processing + network + server). The histogram uses fixed bucket boundaries (~0.26ms granularity at this range), so p99 differences < 0.3ms are bucket noise, not real latency changes. The mean latency confirms no change.

30-Tenant Throughput & Latency

Config Concurrency Protocol main (ops/s) PR (ops/s) Δ ops main p99 (ms) PR p99 (ms) main mean (ms) PR mean (ms)
Read c330t HTTP/1 1,250 1,256 +0.5% 5.48 5.52 2.35 2.35
Read c330t HTTP/2 1,241 1,255 +1.1% 5.56 5.51 2.38 2.34

JFR Allocation Comparison (c128 Read HTTP/1, ObjectAllocationSample)

Class main % PR % Change
HashMap$Node 8.1% 6.8% -1.3pp (-16%)
DefaultHeaders$HeaderEntry 4.2% 4.0% -0.2pp
HashMap$Node[] 3.9% 3.0% -0.9pp (-22%)
HttpHeader 1.7% 1.7%
HeadersUtils$StringEntry 1.5% 1.6%
HashMap 1.4% 1.4%
LinkedHashMap$Entry 1.1% 1.3%
IdentityHashMap$ValueIterator 0.1% 0.0% eliminated
Total targeted ~22.0% ~19.8% -2.2pp

The primary allocation reductions are in HashMap$Node (-16%) and HashMap$Node[] (-22%), reflecting the elimination of resize/rehash cycles from right-sized initial capacity. The IdentityHashMap$ValueIterator is eliminated by removing the asMap() round-trip in StoreResponse.

Per-Round Detail

Click to expand per-round throughput (ops/s)

VM1 (c1, c8, c32-http1):

Config main r1 main r2 main r3 PR r1 PR r2 PR r3
c1-http1 446 438 446 444 448 444
c1-http2 448 453 448 444 451 448
c8-http1 3,719 3,730 3,693 3,718 3,701 3,714
c8-http2 3,692 3,691 3,724 3,699 3,702 3,709
c32-http1 15,079 15,214 15,117 15,170 15,200 15,200

VM2 (c32-http2, c128, 30t):

Config main r1 main r2 main r3 PR r1 PR r2 PR r3
c32-http2 15,505 15,528 15,579 15,422 15,621 15,533
c128-http1 49,994 50,203 50,465 50,085 50,200 51,271
c128-http2 50,069 50,850 50,680 49,858 51,265 50,536
30t-c3-http1 1,255 1,253 1,242 1,252 1,263 1,253
30t-c3-http2 1,233 1,244 1,245 1,254 1,254 1,258

All CVs 1.4% across both refs, confirming stable measurements with pinned endpoints.

Conclusion

  • Throughput: neutral across all 10 configs deltas range from -0.4% to +1.1%, all within measurement noise
  • Latency: p99 and mean latency unchanged across all configs (differences < histogram bucket granularity)
  • Allocation efficiency: HashMap$Node -16%, HashMap$Node[] -22%, IdentityHashMap$ValueIterator eliminated at c128
  • No regression at any concurrency level (c1 through c128) or protocol (HTTP/1, HTTP/2)
  • 30-tenant multi-account: neutral to slightly positive (+0.5% to +1.1%)
  • The changes remove unnecessary allocation overhead without throughput regression

Eliminate per-response intermediate HashMap allocation by adding a new
StoreResponse constructor that accepts HttpHeaders directly. Header names
and values are populated into String[] arrays without materializing an
intermediate Map. The JsonNodeStorePayload is updated to accept header
arrays and only builds a Map lazily on error paths (extremely rare).

Pre-size HashMaps throughout the hot path to avoid resize/rehash:
- HttpHeaders request construction: sized to defaultHeaders + request headers
- StoreResponse.replicaStatusList: pre-sized to 4
- StoreResponse.withRemappedStatusCode: pre-sized to header count
- RxDocumentServiceRequest fallback maps: pre-sized to 32

Fix HttpUtils.asMap() double-allocation by iterating HttpHeaders directly
instead of calling toMap() which creates an intermediate HashMap.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions bot added the Cosmos label Apr 1, 2026
Annie Liang and others added 2 commits April 1, 2026 09:53
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xinlian12 and others added 3 commits April 4, 2026 22:02
…ve null-guard inconsistency

- Fix HashMap<>(4) to HashMap<>(6) for replicaStatusList to avoid rehash
  at 4 replicas (capacity 4 * 0.75 = threshold 3, resizes on 4th insert)
- Refactor JsonNodeStorePayload: extract shared parseJson() method with
  Supplier<Map<String,String>> to eliminate duplicated error-handling logic
- Remove misleading null ternary in getHttpRequestHeaders() since
  getHeaders() always returns non-null (fallback HashMap<>(32))
- Revert HashMap<>(16) to HashMap<>() in HttpHeaders default constructor
  (16 is already the default capacity, change was no-op noise)
- Add unit tests for StoreResponse HttpHeaders constructor, HttpHeaders
  populateLowerCaseHeaders, and JsonNodeStorePayload array-header constructor

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix HashMap<>(headers.size()) in HttpUtils.asMap() to account for the
0.75 load factor, avoiding resize when all headers are inserted.

Extract mapCapacityForSize(int) helper in HttpUtils to consolidate the
capacity calculation (n * 4 / 3 + 1) used across HttpUtils.asMap(),
StoreResponse.withRemappedStatusCode(), and JsonNodeStorePayload.buildHeaderMap().

Addresses review feedback from alzimmermsft.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ize to HttpHeaders, clarify lowercase key guarantee

- Add comments explaining why HashMap<>(32) is used as fallback in
  RxDocumentServiceRequest: capacity 32 gives threshold 24, covering
  typical 15-20 request headers without resize.
- Apply HttpUtils.mapCapacityForSize() in RxGatewayStoreModel.getHttpRequestHeaders()
  to account for 0.75 load factor when constructing HttpHeaders.
- Make mapCapacityForSize() public so it can be used from other packages.
- Document in populateLowerCaseHeaders() Javadoc that keys are guaranteed
  lowercase because HttpHeaders.set() stores them via toLowerCase(Locale.ROOT).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xinlian12 and others added 15 commits April 6, 2026 14:14
Extract duplicate contentStream/payload handling from both StoreResponse
constructors into a shared parseResponsePayload() static helper method.
Both constructors now use the array-based JsonNodeStorePayload constructor,
eliminating code duplication while preserving identical behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove both unescape(Set<Entry>) and unescape(Map) overloads from
  HttpUtils as they are no longer needed
- Update ResponseUtils to use the HttpHeaders-based StoreResponse
  constructor (same optimization as RxGatewayStoreModel)
- Remove unescape test from HttpUtilsTest, keep asMap() coverage
- Clean up unused imports (AbstractMap, ArrayList, List, Set)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace previous benchmark charts with comprehensive V3 analysis:
- 20 configs (c1/c8/c16/c32/c128 x Read/Write x HTTP1/HTTP2)
- 3 rounds each, 10 min/run, GATEWAY mode
- Timeline charts with throughput and P99 latency
- JFR allocation breakdown comparison
- Detailed per-round analysis of outlier patterns

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JFR ObjectAllocationSample weight = estimated cumulative bytes allocated
over the recording (10 min), not heap residency. Heap was 8 GB committed.
The ~271 GB 'targeted' is allocation throughput (~4 GB/s alloc rate),
most objects immediately GC'd.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace confusing cumulative GB with allocation share %
- Add GC comparison table (817 vs 813 pauses - identical)
- Frame as code efficiency improvement, not GC impact

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…arison

- Remove timeline charts (not needed for review)
- Add variance analysis using request-level metrics (transitTime)
  showing variance is server-side, not SDK-related
- Add JFR allocation comparison for all 20 configs
- Keep summary bar chart and c128 JFR chart

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The c16 PR showing HashMap\ is from the request-sending
path (ReactorNettyClient.bodySendDelegate iterating request headers),
NOT the response-side asMap() iterator we eliminated. Added clarifying
note and removed the per-config ValueIterator column (too noisy).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…a-jackson.version

The recent Jackson dependency update (8a671dd) bumped Jackson from 2.18.4
to 2.18.6 in all Cosmos Spark child modules but missed updating the
scala-jackson.version property in the parent POM. This caused the
maven-enforcer-plugin BannedDependencies rule to reject
jackson-module-scala_2.12 and _2.13 at version 2.18.6.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@xinlian12
Copy link
Copy Markdown
Member Author

/azp run java - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

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

LGTM

@FabianMeiswinkel FabianMeiswinkel marked this pull request as ready for review April 15, 2026 10:32
Copilot AI review requested due to automatic review settings April 15, 2026 10:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Optimizes the Cosmos gateway hot path by reducing temporary HashMap/header-collection allocations and removing redundant header normalization.

Changes:

  • Adds capacity sizing helper (HttpUtils.mapCapacityForSize) and applies pre-sizing to avoid HashMap rehash/resizes.
  • Changes StoreResponse to accept HttpHeaders directly and avoids HttpHeaders -> Map -> HttpHeaders round-trips.
  • Adds/updates unit tests for the new header-array population path and new constructors.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/http/HttpHeaders.java Adds populateLowerCaseHeaders to extract headers without building an intermediate map.
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/StoreResponse.java Adds HttpHeaders ctor and factors payload parsing/stream close into a helper; tweaks map sizing.
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/ResponseUtils.java Routes response creation through the new StoreResponse(HttpHeaders, ...) ctor.
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/JsonNodeStorePayload.java Adds ctor that consumes header arrays and lazily builds a map only on error paths.
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/HttpUtils.java Adds mapCapacityForSize, updates asMap, and removes unescape APIs.
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RxGatewayStoreModel.java Avoids lowercasing/map conversions; pre-sizes HttpHeaders for request headers.
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RxDocumentServiceRequest.java Pre-sizes request headers map to 32 to reduce resizes.
sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/http/HttpHeadersTests.java Adds unit tests for populateLowerCaseHeaders.
sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/directconnectivity/StoreResponseTest.java Adds unit tests covering new StoreResponse(HttpHeaders, ...) behavior.
sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/directconnectivity/JsonNodeStorePayloadTests.java Adds unit tests for new header-array JsonNodeStorePayload constructor.
sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/directconnectivity/HttpUtilsTest.java Removes coverage for deleted unescape(Map) API.

Comment on lines +143 to +150
public void populateLowerCaseHeaders(String[] names, String[] values) {
int i = 0;
for (Map.Entry<String, HttpHeader> entry : headers.entrySet()) {
names[i] = entry.getKey();
values[i] = entry.getValue().value();
i++;
}
}
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

populateLowerCaseHeaders is a public method but it relies on a caller-side precondition (arrays must be >= size()) and will otherwise throw ArrayIndexOutOfBoundsException. Consider adding argument validation (e.g., reject null arrays and enforce names.length >= size() and values.length >= size()) and throw an IllegalArgumentException with a clear message to fail fast and make misuse easier to diagnose.

Copilot uses AI. Check for mistakes.
Comment on lines +100 to 108
// URL-decode OWNER_FULL_NAME header value inline (replaces HttpUtils.unescape).
// This is kept separate from populateLowerCaseHeaders because HttpHeaders is a
// general-purpose HTTP class and should not contain Cosmos-specific URL-decoding logic.
for (int i = 0; i < headerCount; i++) {
if (HttpConstants.HttpHeaders.OWNER_FULL_NAME.equals(responseHeaderNames[i])) {
responseHeaderValues[i] = HttpUtils.urlDecode(responseHeaderValues[i]);
break;
}
} else {
this.responsePayload = null;
}
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

This comparison assumes HttpConstants.HttpHeaders.OWNER_FULL_NAME matches the lowercase key stored in responseHeaderNames. Since populateLowerCaseHeaders fills keys from the internal lowercase map, decoding may be skipped if the constant isn’t lowercase. To preserve prior behavior, compare case-insensitively (or compare against a lowercase-normalized constant) so decoding is applied regardless of constant casing.

Copilot uses AI. Check for mistakes.
* without resizing, accounting for the default load factor of 0.75.
*/
public static int mapCapacityForSize(int expectedSize) {
return expectedSize * 4 / 3 + 1;
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

mapCapacityForSize can overflow for large expectedSize values (producing a negative capacity that will fail new HashMap<>(...)) and doesn’t define behavior for negative inputs. Consider guarding expectedSize <= 0 and clamping the computed capacity to a safe upper bound (e.g., Integer.MAX_VALUE) to avoid overflow-driven failures.

Suggested change
return expectedSize * 4 / 3 + 1;
if (expectedSize <= 0) {
return 1;
}
long capacity = (long) expectedSize * 4 / 3 + 1;
return capacity >= Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) capacity;

Copilot uses AI. Check for mistakes.
Comment on lines +42 to +44
for (String name : names) {
assertThat(name).isEqualTo(name.toLowerCase());
}
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

Tests should avoid locale-dependent casing behavior. String#toLowerCase() without an explicit locale can behave unexpectedly under certain default locales (e.g., Turkish). Use toLowerCase(Locale.ROOT) for stable assertions.

Copilot uses AI. Check for mistakes.
logger.warn("Failed to close content stream. This may cause a Netty ByteBuf leak.", e);
}
}
replicaStatusList = new HashMap<>(6);
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

The initial capacity 6 is a magic number repeated across constructors. Consider extracting it into a named constant (or using HttpUtils.mapCapacityForSize(<expectedEntries>) with a brief rationale) so the intent is clearer and future changes don’t require hunting multiple occurrences.

Copilot uses AI. Check for mistakes.
Comment on lines +82 to +87
public StoreResponse(
String endpoint,
int status,
HttpHeaders httpHeaders,
ByteBufInputStream contentStream,
int responsePayloadLength) {
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

The new StoreResponse(HttpHeaders, ...) path moves OWNER_FULL_NAME URL-decoding inline (replacing the removed HttpUtils.unescape(...)). Add a unit test that includes an encoded OWNER_FULL_NAME header and asserts it is decoded when accessed via getHeaderValue, to ensure behavioral parity with the previous map-based flow.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants