Copilot commented on code in PR #11006:
URL: https://github.com/apache/rocketmq/pull/11006#discussion_r3922828912
##########
common/src/main/java/org/apache/rocketmq/common/message/MessageDecoder.java:
##########
@@ -44,6 +44,9 @@ public class MessageDecoder {
public final static int MESSAGE_PHYSIC_OFFSET_POSITION = 28;
public final static int MESSAGE_STORE_TIMESTAMP_POSITION = 56;
+ // Pre-encoded constant to avoid per-message getBytes() allocation in
createCrc32()
+ private static final byte[] PROPERTY_CRC32_BYTES =
MessageConst.PROPERTY_CRC32.getBytes(StandardCharsets.UTF_8);
Review Comment:
Use the existing `CHARSET_UTF8` constant for consistency within
`MessageDecoder` (most `getBytes(...)` calls in this class already use it) and
to avoid hard-coding a second charset reference.
##########
store/src/main/java/org/apache/rocketmq/store/MessageExtEncoder.java:
##########
@@ -82,6 +87,16 @@ public static int calMsgLength(MessageVersion messageVersion,
+ 2 + (Math.max(propertiesLength, 0)); //propertiesLength
}
+ private byte[] getTopicBytes(String topic) {
+ if (topic == cachedTopic || topic.equals(cachedTopic)) {
+ return cachedTopicData;
+ }
+ byte[] data = topic.getBytes(MessageDecoder.CHARSET_UTF8);
+ cachedTopic = topic;
+ cachedTopicData = data;
+ return data;
+ }
Review Comment:
`getTopicBytes` can return `null` when `topic` is `null` and the cache is
uninitialized (`topic == cachedTopic` when both are `null`), which then shifts
the eventual NPE to `topicData.length` and makes the failure mode less direct.
Guard the cache-hit path against `null` so a `null` topic fails consistently at
`topic.getBytes(...)` (same as the pre-cache behavior).
--
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]
