wang-jiahua commented on PR #10640:
URL: https://github.com/apache/rocketmq/pull/10640#issuecomment-5040967646
Thanks for the careful review — both points addressed.
**1. Negative `totalSize` (corrupt length)** — fixed in 198075430.
`checkMessageAndReturnSize` now rejects a negative `totalSize` up front and
returns a failed `DispatchRequest(-1, false)` **before any allocation**,
consistent with the method's other corruption paths (illegal magic code,
insufficient `remaining()`):
```java
int totalSize = byteBuffer.getInt();
if (totalSize < 0 || byteBuffer.remaining() < totalSize - 4) {
return new DispatchRequest(-1, false /* fail */);
}
```
and the now-redundant `totalSize < 0` branch was removed from
`borrowCheckMessageBuffer`. (For accuracy: the original code `new
byte[totalSize]` also threw `NegativeArraySizeException` on a negative length,
so this is a pre-existing corner case — it's just handled gracefully now.)
**2. Benchmark numbers.**
Setup: 4 fully isolated nodes (dedicated Producer / Broker / NameServer /
Consumer), broker `-Xms4g -Xmx4g` G1, Temurin 21; the OS page cache is dropped
and the store is fresh before each arm; 3 interleaved rounds, median reported.
Metric is normalized per message (immune to TPS jitter).
| | Baseline | This PR |
|---|---|---|
| Young GC / million msgs | 9.18 | **8.21 (−10.5%)** |
| P99 send latency | 0.86 ms | 0.85 ms (unchanged) |
| Producer TPS (median) | ~98.9k | ~100.5k (no regression) |
Local micro-measurement (`ThreadMXBean.getThreadAllocatedBytes`, JDK 21) on
this path: **−2320 B/op**.
On the memory concern: the reuse cap is `maxMessageSize + 64K` (default 4 MB
+ 64 K ≈ 4.06 MB), and `checkMessageAndReturnSize` is driven by the single
`ReputMessageService` dispatch thread (plus transient use during recovery/HA),
so the pinned memory is **one buffer per such thread — bounded and small (~4
MB), independent of throughput**.
And you're right that each such allocation is a cheap young-gen object
individually; the measured win is the **reduced young-GC frequency
(−10.5%/msg)** from eliminating this per-message allocation on the dispatch hot
path, rather than a TPS/P99 jump at this operating point.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
