[ISSUE #10813] Reduce temporary allocations in the LMQ append path

[ai-yang]

Which Issue(s) This PR Fixes

• Fixes [Enhancement] Reduce temporary allocations in the LMQ append path #10813

Brief Description

This PR reduces temporary allocation in the LMQ append path without changing public APIs or the persisted message format.

• Build PROPERTY_INNER_MULTI_QUEUE_OFFSET with StringBuilder.append(long) instead of a boxed Long[] plus StringUtils.join.
• Parse the multi-dispatch queue names once on a normal append and reuse them when increasing LMQ offsets.
• Serialize the message properties once on the normal path while preserving the legacy WAIT_STORE_MSG_OK property order and exact encoded bytes.
• Keep queue/offset preparation focused by isolating the legacy WAIT_STORE_MSG_OK remove/reinsert compatibility step in a named helper immediately before final property serialization.
• Keep the queue-name parser private because all of its call sites are internal to LmqDispatch.
• Keep the mapped-file END_OF_FILE retry safe: offsets are increased only after a successful physical append and never twice.
• Preserve the public LmqDispatch methods and existing RocksDB/generic consume-queue exception mapping.

Compatibility

There are no public API, configuration, protocol, or persistence-format changes. Mixed LMQ/non-LMQ queues, disabled LMQ, WAIT_STORE_MSG_OK, and encoded retry behavior remain compatible.

Performance

Paired JMH results on develop at 2daf0e2ca91a1592d18235d43e5d709d1c35d15f:

LMQ queues	Baseline B/op	This PR B/op	Saved B/op	Saved	Median paired throughput change
1	1376.000	704.000	672.000	48.837%	+83.311%
4	3016.001	1512.000	1504.001	49.867%	+106.211%

Environment: OpenJDK 8u492, JMH 1.36, Linux x86_64, Intel Xeon Gold 6133. Five independent alternating baseline-to-patched pairs were rerun for reviewer follow-up commit b28b13f7b. Each variant used one fork per pair, five 1-second warmup iterations, ten 1-second measurement iterations, throughput mode, and -prof gc. The patched harness includes the extracted WAIT compatibility helper immediately before final property serialization. Allocation decreased in 5/5 pairs for both queue counts; the minimum reduction was 672 B/op. Throughput improved in every pair, with a minimum paired gain of 79.374%. The subsequent 245264ce6 review update only narrows the internal parser's access modifier and does not change the benchmarked operations.

How Did You Test This Change?

All Maven gates used JDK 8.

• Reviewer follow-up at b28b13f7b: LmqDispatchTest passed in 20 independent Maven/Surefire processes, 100/100 tests, 0 failures/errors/skips.
• Reviewer follow-up at b28b13f7b: mvn -DskipITs -pl store -am test passed with common 241/241, remoting 174/174, and store 318/318 tests (4 existing skips), 0 failures/errors.
• Reviewer follow-up JMH: both 1-queue and 4-queue scenarios passed all 5/5 allocation and throughput pairs; minimum allocation reduction was 672 B/op and minimum throughput gain was 79.374%.
• Latest visibility follow-up at 245264ce6: LmqDispatchTest passed 5/5 and the full affected reactor passed common 241/241, remoting 174/174, and store 318/318 tests (4 existing skips), with 0 failures/errors; Checkstyle, SpotBugs, store-scoped RAT, and git diff --check also passed.
• mvn -DskipITs -pl '!namesrv' package: all 18 selected reactor modules SUCCESS in 42:07, including store (318 tests), broker, controller (72 tests), proxy (303 tests, 3 existing skips), and container.
• The host already had an unrelated process bound to namesrv port 9876. On the same baseline SHA, an isolated mvn -DskipITs -pl namesrv package passed 116/116 tests. The patched tree also passed namesrv package with tests skipped, Checkstyle 0, and SpotBugs 0. This PR only changes store, which is outside the namesrv dependency graph.
• Affected-reactor Checkstyle: 0 violations.
• Affected-reactor SpotBugs: BugInstance=0, Error=0.
• mvn -DskipTests apache-rat:check: all 19 reactor modules SUCCESS, 0 unapproved/unknown files; the reviewer follow-up also passed a fresh store-scoped RAT check with 0 unapproved/unknown files.
• git diff --check: passed.

Regression coverage includes mixed queue names, disabled LMQ, exact legacy property bytes, successful append, END_OF_FILE retry, exactly-once offset increments, RocksDB failure mapping, and generic consume-queue failure mapping.

[RockteMQ-AI]

Summary

Reduces temporary allocations in the LMQ append path by: (1) building offset strings with StringBuilder.append(long) instead of boxed Long[] + StringUtils.join; (2) parsing multi-dispatch queue names once and reusing them; (3) serializing properties once on the normal path. Good test coverage including a regression test for the original issue.

Findings

• [Info] LmqDispatch.java — The new prepareLmqDispatch method cleanly separates the queue-name resolution from the offset computation. The parseLmqDispatchQueues caching via msgInner.setEncodedBuff is a good optimization.
• [Info] CommitLog.java:1992 — The lmqQueueNames variable is declared outside the if (isMultiDispatchMsg) block and reused in the encode-completed path. This is correct and avoids redundant parsing.

LGTM.

---

Automated review by github-manager-bot

[fuyou001]

Non-blocking readability suggestion: separate LMQ offset preparation from legacy property-order handling

prepareLmqDispatch() currently has two responsibilities: (1) parsing queue names and populating PROPERTY_INNER_MULTI_QUEUE_OFFSET, and (2) removing and reinserting WAIT_STORE_MSG_OK to preserve the legacy serialized property order. The second behavior is an unexpected side effect that is not reflected by the method name and is unrelated to LMQ offset preparation.

Consider extracting the WAIT-order operation into a clearly named helper and invoking it immediately before MessageDecoder.messageProperties2String() in handlePropertiesForLmqMsg(). This would keep prepareLmqDispatch() focused on queue names and offsets while keeping the byte-compatibility logic next to the serialization it affects. The existing exact-legacy-bytes regression assertion should be retained.

[ai-yang]

Thanks @fuyou001 — addressed in b28b13f7b.

• prepareLmqDispatch() now only prepares queue names and offsets.
• The legacy WAIT_STORE_MSG_OK remove/reinsert is isolated in reinsertWaitStorePropertyForLegacySerialization() and invoked immediately before the final messageProperties2String() call.
• The byte-level regression test was renamed to testCommitLogPreservesLegacyPropertyBytesAndIncrementsOffsetOnce to make the compatibility guarantee explicit; its exact legacy property-string and persisted-byte assertions are retained.

Validation on JDK 8: LmqDispatchTest 100/100 across 20 independent Maven processes; affected reactor 733 tests with 0 failures/errors (4 existing skips); Checkstyle, SpotBugs, store-scoped RAT, and git diff --check all passed. I also reran five alternating baseline-to-b28b13f7b JMH pairs with the extracted helper included: allocation decreased and throughput increased in 5/5 pairs for both 1 and 4 queues (minimum 672 B/op saved; minimum throughput gain 79.374%).

[RockteMQ-AI]

Summary

LGTM. Changes look good.

---

Automated review by github-manager-bot

[fuyou001]

Suggestion (non-blocking): parseLmqQueueNames is only used within LmqDispatch. Consider making it private to minimize its visibility and avoid exposing an unnecessary package-private API.

[ai-yang]

Thanks — addressed in 245264ce6 by making parseLmqQueueNames() private; both call sites are internal to LmqDispatch.

Validation on JDK 8: LmqDispatchTest 5/5; full affected reactor 733 tests with 0 failures/errors (4 existing skips); Checkstyle, SpotBugs, store-scoped RAT, and git diff --check passed.

[fuyou001]

LGTM

[RockteMQ-AI]

Summary

Excellent performance optimization for the LMQ append path. The PR achieves ~50% allocation reduction (672-1504 B/op saved) with measurable throughput gains (+83-106%) while maintaining full backward compatibility.

Key strengths:

• Clean refactoring: StringBuilder.append(long) replaces boxed Long[] + StringUtils.join, eliminating autoboxing overhead on the hot path
• Smart reuse: queue names parsed once and passed to updateLmqOffsets, avoiding redundant split() calls
• Correct END_OF_FILE retry handling: offsets only increase after successful physical append, never twice
• Legacy compatibility preserved: reinsertWaitStorePropertyForLegacySerialization clearly documents the remove/reinsert mutation
• Comprehensive test coverage (263 lines): mixed queues, legacy property order, error mapping, retry safety
• Solid JMH methodology: 5 independent alternating pairs, -prof gc, paired throughput analysis

The code is well-structured, the performance gains are real and well-measured, and backward compatibility is carefully maintained. LGTM.

---

Automated review by github-manager-bot