wang-jiahua opened a new issue, #10639:
URL: https://github.com/apache/rocketmq/issues/10639

   ### Before Creating the Enhancement Request
   
   - [x] I have confirmed that this should be classified as an enhancement 
rather than a bug/feature.
   
   ### Summary
   
   `CommitLog.checkMessageAndReturnSize(...)` allocates a fresh `byte[]` the 
size of the **entire message** (`new byte[totalSize]`) on every message it 
parses during reput/dispatch. This scratch array is used purely as a transient 
staging area (fields are copied in and immediately read out) and is discarded 
when the method returns. Replacing it with a per-thread, grow-only reusable 
buffer eliminates this allocation on the hot dispatch path.
   
   ### Motivation
   
   The reput/dispatch thread calls `checkMessageAndReturnSize` once per stored 
message to rebuild the ConsumeQueue/index. The `byte[] bytesContent = new 
byte[totalSize]` inside it is one of the largest byte[] allocators on the store 
path: at high throughput it produces a continuous stream of short-lived, 
whole-message-sized arrays that immediately become garbage, driving up young-GC 
frequency.
   
   Because `bytesContent` is only ever used to copy bytes out of the 
`ByteBuffer` and read them back (CRC check, topic/properties `new String(...)`) 
**within the same call**, and nothing retains a reference to it after the 
method returns, it does not need to be freshly allocated each time.
   
   ### Describe the Solution You'd Like
   
   Borrow the scratch array from a per-thread grow-only buffer instead of 
allocating per call:
   
   - A `ThreadLocal<byte[]>` holds one reusable buffer per dispatch thread (no 
cross-thread sharing, so no concurrency concern).
   - Grow-only: reuse the cached buffer when it is large enough; only allocate 
(and cache) a larger one when a bigger message arrives. All reads/writes use 
explicit lengths, so a larger-than-needed buffer is safe.
   - Safety cap: for an abnormally large message or a corrupt `totalSize` (`> 
maxMessageSize + 64KB`, or `< 0`), fall back to a one-shot `new 
byte[totalSize]` so an oversized buffer is never pinned in the ThreadLocal.
   
   After warmup the buffer reaches the largest message size seen and subsequent 
messages reuse it with zero allocation on this path.
   
   ### Describe Alternatives You've Considered
   
   - **Sizing the array to only the fields actually read (~200B) instead of the 
full message**: more invasive and fragile (the offsets/lengths of 
body/topic/properties vary), and still allocates per message. The ThreadLocal 
reuse removes the allocation entirely with a smaller, localized change.
   - **A pooled/off-heap buffer**: unnecessary complexity for a per-thread 
transient scratch area.
   
   ### Additional Context
   
   Measured effect (allocation reduction, no throughput/latency regression):
   
   - Local micro-measurement (`ThreadMXBean.getThreadAllocatedBytes`, JDK 21): 
**−2320 B/op** on this path.
   - Real-server A/B on a fully isolated 4-node cluster (dedicated Producer / 
Broker / NameServer / Consumer machines; `-Xms4g -Xmx4g` G1; page cache dropped 
before each arm; 3 interleaved rounds, median): broker **young GC per million 
messages −10.5%** (baseline 9.18 → 8.21), **P99 unchanged (~0.85 ms)**, **TPS 
no regression** (median ≥ baseline).
   - Correctness: existing store tests covering `checkMessageAndReturnSize` and 
the CRC paths pass (`AppendPropCRCTest`, `ConsumeQueueTest`).
   
   I have a patch ready and will open a PR referencing this issue.
   


-- 
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]

Reply via email to