[ISSUE #10467] Reduce per-message allocation in createUniqID and valueOfMagicCode#10469
[ISSUE #10467] Reduce per-message allocation in createUniqID and valueOfMagicCode#10469wang-jiahua wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR optimizes message version resolution and reduces allocations when generating unique message IDs.
Changes:
- Replace enum iteration in
MessageVersion#valueOfMagicCodewith direct magic-code comparisons for v1/v2. - Reuse a per-thread
char[]buffer inMessageClientIDSetter#createUniqIDviaThreadLocalto reduce allocations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| common/src/main/java/org/apache/rocketmq/common/message/MessageVersion.java | Switches magic-code lookup from iterating enum values to direct conditional checks. |
| common/src/main/java/org/apache/rocketmq/common/message/MessageClientIDSetter.java | Introduces a ThreadLocal<char[]> buffer to reuse memory during unique ID generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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); |
There was a problem hiding this 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.
| 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(); |
There was a problem hiding this 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.
|
|
||
| public static String createUniqID() { | ||
| char[] sb = new char[LEN * 2]; | ||
| char[] sb = UNIQ_ID_BUF.get(); |
There was a problem hiding this comment.
Good point — renamed sb to buf for clarity.
…d valueOfMagicCode - MessageClientIDSetter.createUniqID(): reuse a ThreadLocal<char[]> instead of allocating a new char[LEN*2] on every send. - MessageVersion.valueOfMagicCode(): replace Enum.values() array copy + O(n) loop with direct if-else on the two known magic codes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
a8e39ce to
a3986cd
Compare
oss-sentinel-ai
left a comment
There was a problem hiding this comment.
Review: Approved ✅
PR: #10469 — Reduce per-message allocation in createUniqID and valueOfMagicCode
Type: Performance (2 files, +12/-10)
Assessment
Optimizes hot path allocations:
- ThreadLocal<char[]> for createUniqID() (eliminates ~405 char[] allocations/60s)
- Direct if-else for valueOfMagicCode() (eliminates Enum.values() array copy)
Verdict
✅ Solid performance optimization based on JFR profiling. Fixes #10467.
🤖 Automated review by oss-sentinel-ai
Which Issue(s) This PR Fixes
Fixes #10467
Brief Description
Two independent per-message allocation reductions:
MessageClientIDSetter.createUniqID(): replacesnew char[LEN * 2]per call with aThreadLocal<char[]>that is allocated once per thread and reused. The char array is only used to build the msgId string within the method and does not escape. JFR shows ~405char[]allocation events/60s from this site eliminated.MessageVersion.valueOfMagicCode(int): replacesEnum.values()(which copies the backing array per JDK spec) + O(n) loop with directif-elsematching on the two known magic code constants (MESSAGE_MAGIC_CODEandMESSAGE_MAGIC_CODE_V2). Eliminates the per-call array copy on the broker dispatch path.Compatibility
createUniqID()produces the same output format — only the internal char buffer allocation strategy changes.valueOfMagicCode()behavior is identical for all valid inputs. Invalid magic codes still throwIllegalArgumentException.How Did You Test This Change