itxaiohanglover commented on PR #10640:
URL: https://github.com/apache/rocketmq/pull/10640#issuecomment-5035866430

   Thanks for the optimization — reusing a per-thread scratch buffer here looks 
reasonable since the contents are consumed before the method returns.
   
   One edge case in `borrowCheckMessageBuffer`: the `totalSize > reuseCap || 
totalSize < 0` branch still does `return new byte[totalSize];`, so a negative 
`totalSize` (e.g. from a corrupt length field) throws 
`NegativeArraySizeException` instead of letting the method surface a 
checksum/error `DispatchRequest`. It may be safer to handle `totalSize < 0` 
explicitly before allocation.
   
   Also, do you have benchmark numbers for this path? Message-sized transient 
allocations are young-gen and usually cheap to collect, so it would help 
reviewers to see the measured win (allocation rate / GC pause / dispatch 
throughput) that justifies pinning up to `maxMessageSize + 64K` per dispatch 
thread.


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