perf: reduce HashMap/collection allocation overhead in gateway path

[xinlian12]

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.

[github-actions]

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• releaseassets.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "releaseassets.githubusercontent.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

• pr:#45 (pull_request_read: Resource 'pr:#45' has lower integrity than agent requires. Agent would need to drop integrity tags [unapproved:all approved:all] to trust this resource.)

Generated by sdkReviewAgent for issue #45

[github-actions]

🟡 Recommendation · Correctness: HashMap<>(4) undersizes for standard replica factor

new HashMap<>(4) gives an internal threshold of 3 (capacity × 0.75). In a standard 4-replica cluster every error path that adds one entry per replica will trigger a resize on the 4th insertion — the opposite of the stated pre-sizing goal.

Use new HashMap<>(6) (threshold 4) to fit 4 replicas without rehashing:

replicaStatusList = new HashMap<>(6);

Same applies to the identical lines 72 and 163 in this file.

---

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

[github-actions]

🟡 Recommendation · Code Quality: Full happy-path duplication between fromJsonWithArrayHeaders and fromJson

Lines 96–99 are an exact copy of the happy path in fromJson (lines 60–63). The only real difference is how responseHeaders is obtained in the catch block (lazily built from arrays vs. passed directly). If the parsing logic ever changes (e.g., streaming parse, byte-count validation, or a new encoding branch), the two methods will silently diverge.

Consider extracting the shared parsing and error-handling into a single method that accepts a Supplier(Map(String,String)) for the lazy headers:

private static JsonNode parseJson(
        ByteBufInputStream bufferStream,
        int readableBytes,
        Supplier(Map(String, String)) headersSupplier) {
    byte[] bytes = new byte[readableBytes];
    try {
        bufferStream.read(bytes);
        return Utils.getSimpleObjectMapper().readTree(bytes);
    } catch (IOException e) {
        Map(String, String) responseHeaders = headersSupplier.get();
        if (fallbackCharsetDecoder != null) { ... }
        ...
    }
}

Then fromJson passes () -> responseHeaders (already materialized) and the new array-based path passes () -> buildHeaderMap(headerNames, headerValues) (lazy). This eliminates the duplication while preserving the allocation benefit.

---

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

[github-actions]

🟡 Recommendation · Correctness: Inconsistent null guard — pre-sizing is null-safe, but containsKey is not

Line 346 adds (headers != null ? headers.size() : 0), implying null is a valid argument. But line 349 calls headers.containsKey(...) unconditionally, which would throw NullPointerException if headers is ever null.

The RxDocumentServiceRequest changes in this PR (new HashMap<>(32) as a fallback) ensure getHeaders() is non-null in the only call site today — but the method signature accepts Map(String, String) so future callers could pass null.

Option A — make the contract explicit (callers guarantee non-null):

// Remove the ternary; always use headers.size()
HttpHeaders httpHeaders = new HttpHeaders(this.defaultHeaders.size() + headers.size());

Option B — make the method defensively correct for null:

if (!headers.containsKey(entry.getKey())) {

→

if (headers == null || !headers.containsKey(entry.getKey())) {

Option A is preferable given that getHeaders() is now always non-null, and it avoids the misleading implication that null is an accepted input.

---

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

[github-actions]

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• releaseassets.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "releaseassets.githubusercontent.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

• pr:#45 (pull_request_read: Resource 'pr:#45' has lower integrity than agent requires. Agent would need to drop integrity tags [unapproved:all approved:all] to trust this resource.)

Generated by sdkReviewAgent for issue #45

[github-actions]

🟡 Recommendation · Test Coverage: New hot-path constructor and helper have no tests

Three newly added code paths have zero test coverage:

1. StoreResponse(endpoint, status, HttpHeaders, …) — the new hot-path constructor
2. HttpHeaders.populateLowerCaseHeaders(names, values) — called right here
3. JsonNodeStorePayload(bufferStream, readableBytes, String[], String[]) error path (where buildHeaderMap is lazily invoked)

What to add:

• A unit test asserting that StoreResponse(HttpHeaders) and StoreResponse(Map) produce identical responseHeaderNames/responseHeaderValues arrays (including lowercase normalization and the OWNER_FULL_NAME URL-decode).
• A unit test for populateLowerCaseHeaders verifying that names in the output array are lowercase even when headers were set with mixed-case names.
• An error-path test for JsonNodeStorePayload(…, String[], String[]) that injects invalid JSON and asserts the lazily-built header map is correct inside the thrown exception.

Without these, the main perf optimization (the new constructor) can silently regress or diverge from the Map-based constructor if HttpHeaders internals change.

---

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

[github-actions]

💬 Observation · Code Quality: HashMap<>(16) is identical to HashMap<>()

HashMap's no-arg constructor already uses a default initial capacity of 16 (DEFAULT_INITIAL_CAPACITY = 1 << 4). Changing new HashMap<>() to new HashMap<>(16) has no runtime effect.

If the intent is to document the allocation size for clarity, a comment is clearer. If the intent is an actual optimisation, it needs a different initial capacity (e.g., new HashMap<>(32) for headers-heavy flows). As written this change adds visual noise without benefit.

---

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}