[I] [Enhancement] Add getMessageType(Map) overload to eliminate redundant properties decode in send path [rocketmq]

["wang-jiahua (via GitHub)" <gi...@apache.org>]

wang-jiahua opened a new issue, #10486:
URL: https://github.com/apache/rocketmq/issues/10486

   ### Before Creating the Enhancement Request
   
   - [x] I have confirmed that this should be classified as an enhancement rather than a bug/feature.
   
   ### Summary
   
   `BrokerMetricsManager.getMessageType(SendMessageRequestHeader)` is called once per send to classify the message (`NORMAL`/`TRANSACTION`/`DELAY`/`FIFO`). It internally decodes the properties String into a `HashMap` by calling `MessageDecoder.string2messageProperties(...)` — but in the typical send path, the caller `SendMessageProcessor` has already decoded the same properties String just before calling this method (e.g. in `buildMsgContext`). The result is a redundant decode on every send: a fresh `HashMap` (with associated `Node[]`, `String` substrings, etc.) is allocated, queried for 5 keys, and immediately discarded.
   
   This issue proposes adding a `getMessageType(Map<String, String>)` overload that lets callers pass an already-decoded `Map` and reuse it.
   
   ### Motivation
   
   Per-send call chain in `SendMessageProcessor`:
   
   ```
   SendMessageProcessor.sendMessage(request)
     ├─ buildMsgContext(request)
     │    └─ string2messageProperties(propStr)  [decode #1]  → Map A (used for hook context, msgInner.properties)
     │    └─ BrokerMetricsManager.incTopicMessage(...)
     │         └─ getMessageType(header)
     │              └─ string2messageProperties(propStr)  [decode #2]  → Map B (used to read 5 keys, then discarded)
   ```
   
   JFR sampling on broker shows `string2messageProperties` accounts for 357 samples on the send path; ~half of those are this redundant second decode. A typical message has ~14 properties, so each redundant decode allocates one `HashMap` + ~14 `String` substrings + 1 `Node[]` (size 32 after PR #10444's right-sizing, or 128 before).
   
   ### Describe the Solution You'd Like
   
   Add a public overload on `BrokerMetricsManager`:
   
   ```java
   public static TopicMessageType getMessageType(Map<String, String> properties) {
       // existing 5-key lookup logic, but reading from the passed-in Map
       ...
   }
   
   // keep the old overload for callers that don't already have a decoded Map
   public static TopicMessageType getMessageType(SendMessageRequestHeader requestHeader) {
       return getMessageType(MessageDecoder.string2messageProperties(requestHeader.getProperties()));
   }
   ```
   
   The downstream `SendMessageProcessor` change (passing the already-decoded Map) lives in a separate broker-layer commit and is not part of this PR; this PR only introduces the overload itself.
   
   ### Describe Alternatives You've Considered
   
   - **Cache the decoded Map on `RequestHeader`**: rejected because `RemotingCommand` is the wire object, not a decode cache; mutating it for this purpose would muddy the layering.
   - **Skip the change and accept the redundant decode**: 100k QPS × ~16 entry HashMap allocation is several MB/s of allocation — non-trivial, especially after PR #10444 reduces baseline allocation rate.
   
   ### Additional Context
   
   - Related: issue #10441 (parent issue covering the broader metrics allocation reduction).
   - This change is independent of #10443 / #10481 — the new overload doesn't reference any new constants.
   


-- 
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: commits-unsubscribe@rocketmq.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Enhancement] Add getMessageType(Map) overload to eliminate redundant properties decode in send path [rocketmq]

["RockteMQ-AI (via GitHub)" <gi...@apache.org>]

RockteMQ-AI commented on issue #10486:
URL: https://github.com/apache/rocketmq/issues/10486#issuecomment-4679389451

   ## 🤖 Issue Evaluation
   
   **Classification:** Enhancement (Performance)
   **Priority:** Medium
   **Status:** ✅ Valid Enhancement
   
   ### Assessment
   
   Well-identified performance issue. The redundant `MessageDecoder.string2messageProperties(...)` call on every send creates unnecessary HashMap allocations that are immediately discarded after querying 5 keys.
   
   **Analysis:**
   - `BrokerMetricsManager.getMessageType(SendMessageRequestHeader)` decodes properties String internally
   - `SendMessageProcessor.buildMsgContext` already decoded the same properties String just before
   - Result: redundant HashMap + Node[] + String allocations per message send
   
   **Recommendation:**
   Adding a `getMessageType(Map<String, String>)` overload is a clean solution. The caller can pass the already-decoded properties map, eliminating the redundant decode.
   
   This is a straightforward optimization with clear benefit on the send hot path.
   
   **Next Steps:**
   - PR welcome — the change is localized to `BrokerMetricsManager` and its callers
   - Should include JMH or JFR before/after comparison to quantify the allocation reduction
   
   ---
   *🤖 Automated evaluation by oss-sentinel-ai*


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Enhancement] Add getMessageType(Map) overload to eliminate redundant properties decode in send path [rocketmq]

["RongtongJin (via GitHub)" <gi...@apache.org>]

RongtongJin closed issue #10486: [Enhancement] Add getMessageType(Map) overload to eliminate redundant properties decode in send path
URL: https://github.com/apache/rocketmq/issues/10486


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org