You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "Gautam-aman (via GitHub)" <gi...@apache.org> on 2026/06/27 16:00:53 UTC
[PR] [ISSUE #10488] Optimize gRPC message body conversion with zero-copy ByteString [rocketmq]
Gautam-aman opened a new pull request, #10555:
URL: https://github.com/apache/rocketmq/pull/10555
## 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10488] Optimize gRPC message body conversion with zero-copy ByteString [rocketmq]
Posted by "Gautam-aman (via GitHub)" <gi...@apache.org>.
Gautam-aman commented on PR #10555:
URL: https://github.com/apache/rocketmq/pull/10555#issuecomment-4819369428
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10488] Optimize gRPC message body conversion with zero-copy ByteString [rocketmq]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10555:
URL: https://github.com/apache/rocketmq/pull/10555#issuecomment-4819124203
## [Codecov](https://app.codecov.io/gh/apache/rocketmq/pull/10555?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 48.17%. Comparing base ([`10d498c`](https://app.codecov.io/gh/apache/rocketmq/commit/10d498c68aaea5c15ca844d3b3b2d257847d1875?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`858824a`](https://app.codecov.io/gh/apache/rocketmq/commit/858824aae6d482f513cc09a7d5742cf9e912d5dc?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #10555 +/- ##
=============================================
- Coverage 48.26% 48.17% -0.09%
+ Complexity 13427 13409 -18
=============================================
Files 1378 1378
Lines 100822 100822
Branches 13036 13036
=============================================
- Hits 48658 48576 -82
- Misses 46217 46282 +65
- Partials 5947 5964 +17
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/rocketmq/pull/10555?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10488] Optimize gRPC message body conversion with zero-copy ByteString [rocketmq]
Posted by "RockteMQ-AI (via GitHub)" <gi...@apache.org>.
RockteMQ-AI commented on PR #10555:
URL: https://github.com/apache/rocketmq/pull/10555#issuecomment-4832597837
Thanks for the detailed clarification @qianye1001 — you're absolutely right. The previous review comment incorrectly stated that `Message.build()` ensures consumption before mutation. As you verified, `build()` does **not** copy or consume the `ByteString`; the protobuf `Message` continues to alias the original `byte[]` via `UnsafeByteOperations.unsafeWrap()`.
Your analysis of the receive path is correct: `MessageDecoder.decode()` allocates a fresh body array, and downstream code only touches receipt/properties — not the body bytes — so the current usage is safe in practice.
Agreed that the inline comment should document the **actual** invariant rather than the incorrect one. The real safety guarantee is:
> The decoded body array must not be mutated or reused while the returned protobuf message may still be referenced or serialized.
@Gautam-aman — could you update the safety comment in `GrpcConverter.java` to reflect this invariant? Something along the lines of:
```java
// Safety: UnsafeByteOperations.unsafeWrap() aliases the source byte[] —
// the protobuf Message does NOT copy it. The caller must ensure the
// decoded body array is never mutated or reused while the Message
// (or any serialized form of it) may still be in use.
```
---
*Automated review by ${BOT_SIG}*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10488] Optimize gRPC message body conversion with zero-copy ByteString [rocketmq]
Posted by "RockteMQ-AI (via GitHub)" <gi...@apache.org>.
RockteMQ-AI commented on PR #10555:
URL: https://github.com/apache/rocketmq/pull/10555#issuecomment-4820143646
Thanks for the quick update! The indentation is now consistent and the safety comment clearly documents the `unsafeWrap()` assumptions. Changes look good. 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10488] Optimize gRPC message body conversion with zero-copy ByteString [rocketmq]
Posted by "Gautam-aman (via GitHub)" <gi...@apache.org>.
Gautam-aman commented on PR #10555:
URL: https://github.com/apache/rocketmq/pull/10555#issuecomment-4841562001
Thanks for the clarification. Updated the safety comment to reflect the actual `unsafeWrap()` invariant and buffer lifetime requirements.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10488] Optimize gRPC message body conversion with zero-copy ByteString [rocketmq]
Posted by "qianye1001 (via GitHub)" <gi...@apache.org>.
qianye1001 commented on PR #10555:
URL: https://github.com/apache/rocketmq/pull/10555#issuecomment-4831896085
One note on the safety comment: `Message.build()` does not consume or copy the `ByteString`. With `UnsafeByteOperations.unsafeWrap(messageExt.getBody())`, the built protobuf `Message` still aliases the original `byte[]`.
I verified this with a small local probe: mutating the source body after `buildMessage()` is reflected in `grpcMessage.getBody()`.
The current receive path still looks safe to me because `MessageDecoder.decode()` allocates a fresh body array, and the later receive-path code only updates receipt/properties, not the body bytes. So I do not see this as a blocker, but I suggest adjusting the comment to document the real invariant: the decoded body array must not be mutated or reused while the returned protobuf message may still be used or serialized.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10488] Optimize gRPC message body conversion with zero-copy ByteString [rocketmq]
Posted by "Gautam-aman (via GitHub)" <gi...@apache.org>.
Gautam-aman commented on PR #10555:
URL: https://github.com/apache/rocketmq/pull/10555#issuecomment-4842060717
All Maven and integration checks are passing, including `LocalGrpcIT`. The remaining Bazel failure appears to be a timeout in `PopConsumerServiceTest`, which looks unrelated to this gRPC optimization. I'm checking the Bazel test log for more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10488] Optimize gRPC message body conversion with zero-copy ByteString [rocketmq]
Posted by "Gautam-aman (via GitHub)" <gi...@apache.org>.
Gautam-aman commented on PR #10555:
URL: https://github.com/apache/rocketmq/pull/10555#issuecomment-4842104262
The remaining Bazel failure appears to be a timeout in `PopConsumerServiceTest`. All Maven builds, integration tests, and `LocalGrpcIT` are passing. The timeout log does not show any assertion or exception related to this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org