[ISSUE #9926] Avoid redundant list allocation in parsePublishMessageQueues#10380
Conversation
…ssageQueues When namespace is empty (the common case for most users), the method previously allocated a new ArrayList and N new MessageQueue objects on every send call, copying identical data. Under high throughput (50K+ TPS), this creates significant GC pressure from short-lived objects. This change: - Returns the original list directly when namespace is empty (zero alloc) - Pre-allocates ArrayList capacity when namespace stripping is needed Signed-off-by: sywu14 <wushiyuanwork@outlook.com>
| List<MessageQueue> resultQueues = new ArrayList<>(); | ||
| String namespace = this.mQClientFactory.getClientConfig().getNamespace(); | ||
| if (namespace == null || namespace.isEmpty()) { | ||
| return messageQueueList; |
There was a problem hiding this comment.
One concern about this change is that the method semantics change from “always returning a new list” to “returning the original list when namespace is empty”.
If any caller mutates the returned list, this could accidentally mutate the original message queue list, especially if that list is reused or cached elsewhere in the send path.
Could you confirm that the return value of parsePublishMessageQueues is treated as read-only by all callers, or that the original list is not reused as mutable shared state?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #10380 +/- ##
=============================================
- Coverage 48.89% 48.83% -0.07%
+ Complexity 13452 13439 -13
=============================================
Files 1376 1376
Lines 100527 100530 +3
Branches 12983 12984 +1
=============================================
- Hits 49154 49090 -64
- Misses 45383 45425 +42
- Partials 5990 6015 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Which Issue(s) This PR Fixes
parsePublishMessageQueues#9926Brief Description
MQAdminImpl.parsePublishMessageQueues()is called on every message send (hot path). Under high throughput (50K+ TPS), it creates significant GC pressure by allocating a newArrayListand N newMessageQueueobjects per call — even when namespace is empty (the common case for most users), where the copied data is identical to the original.Changes:
ArrayListwith exact capacity to avoid internal array resizinggetNamespace()calls in the loopImpact: Eliminates per-message object allocation on the send path for the majority of users who don't use namespaces. For users with namespaces, reduces allocation by pre-sizing the list.
How Did You Test This Change?
assertParsePublishMessageQueuesReturnsOriginalListWhenNoNamespace— verifies same list instance is returned when namespace is emptyassertParsePublishMessageQueuesStripsNamespace— verifies namespace stripping still works correctly