Fix JVM deoptimization loop due to lambda misprediction

[itschrispeck]

Description

This change resolves a JVM deoptimization loop by refactoring how primaryResponse is written to the stream.

To do this, we explicitly check if primaryResponse == null, avoiding the check within the lambda. This prevents the C2 compiler from generating an invokedynamic instruction, which is prone to permanent deoptimization when branch traffic flip-flops. The explicit if/else allows the JVM to compile both paths, relying on CPU branch prediction instead of a JVM trap.

It also fixes a potential race condition where the volatile primaryResponse is read once to check, and read again for use, though it may be modified in between.

This pulls the writeOptionalWriteable logic outside, but this follows the convention of other places in the repo.

Background

We recently found elevated CPU despite no change in load.

Profiling revealed it was due to a deopt/uncommon trap hit at BulkItemRequest#writeThin

This is an example deopt event from the profile:

jdk.Deoptimization {
  startTime = 19:11:35.451 (2026-03-10)
  compileId = 26645
  compiler = "c2"
  method = org.opensearch.action.bulk.BulkItemRequest.writeThin(StreamOutput)
  lineNumber = 142
  bci = 21
  instruction = "ifnonnull"
  reason = "unstable_if"
  action = "none"
  eventThread = "opensearch[esdock-opensearch-metric-mvp-production-cluster-phx-ginuz-sukam][write][T#15]" (javaThreadId = 267)
  stackTrace = [
    org.opensearch.action.bulk.BulkItemRequest.writeThin(StreamOutput) line: 142
    org.opensearch.action.bulk.BulkShardRequest.lambda$writeTo$3(StreamOutput, BulkItemRequest) line: 102
    org.opensearch.core.common.io.stream.StreamOutput.writeArray(Writeable$Writer, Object[]) line: 939
    org.opensearch.action.bulk.BulkShardRequest.writeTo(StreamOutput) line: 99
    org.opensearch.action.support.replication.TransportReplicationAction$ConcreteShardRequest.writeTo(StreamOutput) line: 1589
  ]
}

action = "none" implies that we've hit the recompilation cutoff for the method. Since lambdas use invokedynamic, my understanding is that it's possible to flip-flop between predictions, eventually hitting the cutoff. In this case, the cutoff was hit while the wrong branch was compiled, and each invocation hits the trap.

Related Issues

n/a

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 89f0b71)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review API Mismatch The new call out.writeOptionalWriteable((o, resp) -> resp.writeThin(o), primaryResponse) uses a two-argument overload of writeOptionalWriteable. It should be verified that this overload exists in StreamOutput and behaves identically to the single-argument version (i.e., correctly handles the null case by writing a boolean false before the value). If this overload does not exist or has different null-handling semantics, this could introduce a serialization bug or compilation error. out.writeOptionalWriteable((o, resp) -> resp.writeThin(o), primaryResponse);

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 89f0b71
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix incorrect method signature for writeOptionalWriteable The writeOptionalWriteable method typically accepts a single Writeable argument, not a lambda with a separate object. The original code used a method reference primaryResponse::writeThin which correctly implements the Writeable interface. The new signature may not match the actual API, causing a compilation error or incorrect behavior. server/src/main/java/org/opensearch/action/bulk/BulkItemRequest.java [144] -out.writeOptionalWriteable((o, resp) -> resp.writeThin(o), primaryResponse); +out.writeOptionalWriteable(primaryResponse == null ? null : primaryResponse::writeThin); Suggestion importance[1-10]: 7 __ Why: The suggestion raises a valid concern about the writeOptionalWriteable method signature. The new code uses a two-argument lambda form (o, resp) -> resp.writeThin(o) with a separate primaryResponse argument, which may not match the actual API. The original approach using a conditional method reference primaryResponse == null ? null : primaryResponse::writeThin is a well-established pattern that correctly implements the Writeable interface.	Medium

---

Previous suggestions

Suggestions up to commit 9887b0e

Category	Suggestion	Impact
Possible issue	Verify serialization format compatibility with deserialization The original code used out.writeOptionalWriteable() which handles the null-check and boolean flag internally in a consistent way. The new implementation manually writes a boolean flag followed by the object, but this must exactly match how writeOptionalWriteable encodes data, otherwise deserialization will break. Verify that the reading side uses a matching readOptionalWriteable or equivalent that expects this exact boolean+data format. server/src/main/java/org/opensearch/action/bulk/BulkItemRequest.java [146-151] if (primaryResponse != null) { out.writeBoolean(true); primaryResponse.writeThin(out); } else { out.writeBoolean(false); } +// Ensure the reading side uses readBoolean() + conditional readObject(), NOT readOptionalWriteable() Suggestion importance[1-10]: 5 __ Why: This is a valid concern about serialization compatibility - the new manual boolean+data format must match the reading side's deserialization logic. However, the improved_code is essentially the same as existing_code with just a comment added, making it more of a verification request than an actual code fix.	Low

[github-actions]

❌ Gradle check result for 9887b0e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

✅ Gradle check result for 9887b0e: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.26%. Comparing base (8f8f7b5) to head (89f0b71).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #20832      +/-   ##
============================================
- Coverage     73.31%   73.26%   -0.06%     
  Complexity    72248    72248              
============================================
  Files          5795     5795              
  Lines        330044   330057      +13     
  Branches      47641    47642       +1     
============================================
- Hits         241975   241806     -169     
- Misses        68609    68900     +291     
+ Partials      19460    19351     -109     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Persistent review updated to latest commit 89f0b71

[github-actions]

❌ Gradle check result for 89f0b71: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

✅ Gradle check result for 89f0b71: SUCCESS