wang-jiahua commented on code in PR #10469:
URL: https://github.com/apache/rocketmq/pull/10469#discussion_r3389204462
##########
common/src/main/java/org/apache/rocketmq/common/message/MessageVersion.java:
##########
@@ -71,12 +71,12 @@ public void putTopicLength(ByteBuffer buffer, int
topicLength) {
}
public static MessageVersion valueOfMagicCode(int magicCode) {
- for (MessageVersion version : MessageVersion.values()) {
- if (version.getMagicCode() == magicCode) {
- return version;
- }
+ if (magicCode == MessageDecoder.MESSAGE_MAGIC_CODE) {
+ return MESSAGE_VERSION_V1;
+ }
+ if (magicCode == MessageDecoder.MESSAGE_MAGIC_CODE_V2) {
+ return MESSAGE_VERSION_V2;
}
-
throw new IllegalArgumentException("Invalid magicCode " + magicCode);
Review Comment:
Intentional trade-off. A static lookup map built from `values()` would
achieve the same result but adds complexity for exactly two known constants.
The if-else is trivially verifiable and a new version would just add one more
line. The `values()` call this replaces was the problem — it copies the enum
backing array on every invocation (JDK spec), which this fix eliminates.
##########
common/src/main/java/org/apache/rocketmq/common/message/MessageClientIDSetter.java:
##########
@@ -111,8 +111,10 @@ public static int getPidFromID(String msgID) {
return value & 0x0000FFFF;
}
+ private static final ThreadLocal<char[]> UNIQ_ID_BUF =
ThreadLocal.withInitial(() -> new char[LEN * 2]);
+
public static String createUniqID() {
- char[] sb = new char[LEN * 2];
+ char[] sb = UNIQ_ID_BUF.get();
Review Comment:
The retained buffer is `LEN * 2` bytes (~32–64 bytes depending on IP
version) per thread — negligible even with hundreds of threads. RocketMQ
producer threads are long-lived pool threads, so the ThreadLocal lifecycle
aligns with thread lifecycle. Added a brief inline comment noting the
per-thread retention.
##########
common/src/main/java/org/apache/rocketmq/common/message/MessageClientIDSetter.java:
##########
@@ -111,8 +111,10 @@ public static int getPidFromID(String msgID) {
return value & 0x0000FFFF;
}
+ private static final ThreadLocal<char[]> UNIQ_ID_BUF =
ThreadLocal.withInitial(() -> new char[LEN * 2]);
+
public static String createUniqID() {
- char[] sb = new char[LEN * 2];
+ char[] sb = UNIQ_ID_BUF.get();
Review Comment:
Good point — renamed `sb` to `buf` for clarity.
--
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]
