[ISSUE #10488] Optimize gRPC message body conversion with zero-copy ByteString

[Gautam-aman]

What is changed

Replace ByteString.copyFrom(messageExt.getBody()) with UnsafeByteOperations.unsafeWrap(messageExt.getBody()) in GrpcConverter.buildMessage().

Why

On the Proxy receive path, the message body has already been materialized as a dedicated byte[] during decoding.

ByteString.copyFrom() performs an additional allocation and array copy before gRPC serialization. Using UnsafeByteOperations.unsafeWrap() allows the existing buffer to be wrapped directly, eliminating an unnecessary copy.

Impact

• Removes one heap allocation per received message.
• Avoids an extra System.arraycopy.
• Reduces GC pressure on high-throughput consumer workloads.

Compatibility

No behavioral changes. The resulting ByteString contents remain identical. Only the internal copy is removed.

Verification

• Verified the change is limited to GrpcConverter.buildMessage().
• Checkstyle passes successfully.
• UnsafeByteOperations is available through the existing protobuf dependency.

Closes #10488.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.17%. Comparing base (10d498c) to head (409f51d).

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10555      +/-   ##
=============================================
- Coverage      48.26%   48.17%   -0.09%     
+ Complexity     13427    13402      -25     
=============================================
  Files           1378     1378              
  Lines         100822   100822              
  Branches       13036    13036              
=============================================
- Hits           48658    48570      -88     
- Misses         46217    46285      +68     
- Partials        5947     5967      +20     

☔ View full report in Codecov by Harness.
📢 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.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Replaces with in to eliminate an unnecessary array copy on the Proxy receive path.

Findings

• [Warning] — Indentation misalignment. The line has 16 spaces of indentation while the other builder chain calls (, , , ) use 12 spaces. Please align to 12 spaces to maintain consistent formatting:

• [Info] — ** safety note.** wraps the array without copying, meaning the shares the underlying buffer. This is safe here because returns a dedicated from deserialization that is not mutated downstream. However, if future code changes cause this array to be shared or modified after , it could lead to silent data corruption. Consider adding a brief inline comment (e.g., ) to document this assumption for future maintainers.

Suggestions

1. Fix the indentation on line 105 (16 → 12 spaces).
2. Optionally add an inline comment documenting the safety assumption.

The optimization itself is valid — it removes one heap allocation and one per received message, reducing GC pressure on high-throughput consumer workloads.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot

Summary

Replaces ByteString.copyFrom() with UnsafeByteOperations.unsafeWrap() in GrpcConverter.buildMessage() to eliminate an unnecessary array copy on the Proxy receive path.

Findings

• [Warning] GrpcConverter.java:105 — Indentation misalignment. The .setBody(...) line has 16 spaces of indentation while the other builder chain calls (.setTopic(), .putAllUserProperties(), .setSystemProperties(), .build()) use 12 spaces. Please align to 12 spaces to maintain consistent formatting:

  return Message.newBuilder()
      .setTopic(topic)
      .putAllUserProperties(userProperties)
      .setSystemProperties(systemProperties)
      .setBody(UnsafeByteOperations.unsafeWrap(messageExt.getBody()))
      .build();

• [Info] GrpcConverter.java:105 — unsafeWrap safety note. UnsafeByteOperations.unsafeWrap(byte[]) wraps the array without copying, meaning the ByteString shares the underlying buffer. This is safe here because messageExt.getBody() returns a dedicated byte[] from deserialization that is not mutated downstream. However, if future code changes cause this array to be shared or modified after buildMessage(), it could lead to silent data corruption. Consider adding a brief inline comment (e.g., // safe: body byte[] is not mutated after deserialization) to document this assumption for future maintainers.

Suggestions

1. Fix the indentation on line 105 (16 spaces to 12 spaces).
2. Optionally add an inline comment documenting the unsafeWrap safety assumption.

The optimization itself is valid — it removes one heap allocation and one System.arraycopy per received message, reducing GC pressure on high-throughput consumer workloads.

---

Automated review by github-manager-bot

[Gautam-aman]

Thanks for the review.

I've updated the indentation to match the existing builder chain formatting and added a comment documenting the safety assumption behind UnsafeByteOperations.unsafeWrap().

The wrapped array originates from message deserialization as a dedicated byte[] and is not mutated after conversion on the receive path.

[RockteMQ-AI]

Thanks for the quick update! The indentation is now consistent and the safety comment clearly documents the unsafeWrap() assumptions. Changes look good. 👍

[RockteMQ-AI]

Review by github-manager-bot (Re-review after new commit)

Summary

Re-review of the updated diff. Commit 409f51da improves the inline safety comment for UnsafeByteOperations.unsafeWrap().

Changes Since Last Review

The safety comment was expanded from a 2-line note to a structured 4-line explanation covering:

1. getBody() returns a dedicated array (not shared buffer)
2. Message.build() is called immediately, ensuring consumption before any potential mutation

Findings

• [Info] GrpcConverter.java:105-108 — Good improvement. The expanded comment clearly documents the unsafeWrap safety assumptions for future maintainers. This addresses the previous suggestion well.

• [Info] GrpcConverter.java:99-110 — Indentation is now consistent. All builder chain calls use 16-space indentation (4 levels). Previous warning is resolved.

Verdict

Both previous findings are addressed. The change is clean, focused, and safe. No new issues found.

---

Automated review by github-manager-bot