[ISSUE #10486] Add getMessageType(Map) overload to eliminate redundant properties decode by wang-jiahua · Pull Request #10487 · apache/rocketmq

wang-jiahua · 2026-06-11T09:42:06Z

Which Issue(s) This PR Fixes

Brief Description

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, but the typical caller (SendMessageProcessor) has already decoded the same String moments before. The result is a redundant decode allocation per send (one HashMap + ~14 String substrings + one Node[]).

This PR adds a public overload getMessageType(Map<String, String>) that lets callers pass an already-decoded Map and reuse it. The existing SendMessageRequestHeader overload now delegates to the new overload; behavior is unchanged for callers that don't have a decoded Map.

// new public overload
public static TopicMessageType getMessageType(Map<String, String> properties) { ... }

// existing overload now delegates
public static TopicMessageType getMessageType(SendMessageRequestHeader requestHeader) {
    return getMessageType(MessageDecoder.string2messageProperties(requestHeader.getProperties()));
}

Downstream callers (e.g. SendMessageProcessor) can switch to the new overload in a separate broker-layer commit.

How Did You Test This Change?

Existing BrokerMetricsManagerTest (27 tests) passes. The behavior of the original overload is preserved by delegation.
The new overload uses the exact same key-lookup logic as before, only sourced from a passed-in Map instead of a freshly decoded one.

Independence

This PR is independent of #10443 and #10481 — it doesn't reference any new constants and only restructures BrokerMetricsManager.getMessageType(...) (1 file, +4/-1).

…dundant properties decode BrokerMetricsManager.getMessageType(SendMessageRequestHeader) is called once per send to classify the message. It internally decodes the properties String into a HashMap, but the typical caller (SendMessageProcessor) has already decoded the same String moments before. The result is a redundant decode allocation per send (one HashMap + ~14 String substrings + one Node[]). This commit adds a public overload getMessageType(Map<String, String>) that lets callers pass an already-decoded Map and reuse it. The existing SendMessageRequestHeader overload now delegates to the new overload; behavior is unchanged for callers that don't have a decoded Map. Downstream callers (e.g. SendMessageProcessor) can switch to the new overload in a separate broker-layer commit.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Refactors message-type detection to centralize logic based on parsed message properties, improving reuse and reducing duplication.

Changes:

Changes getMessageType(SendMessageRequestHeader) to delegate after parsing properties.
Introduces an overload getMessageType(Map<String, String>) containing the core logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov-commenter · 2026-06-11T12:30:13Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.02%. Comparing base (82a6a78) to head (cb4ff2e).
⚠️ Report is 15 commits behind head on develop.

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10487      +/-   ##
=============================================
- Coverage      48.06%   48.02%   -0.05%     
+ Complexity     13313    13300      -13     
=============================================
  Files           1377     1377              
  Lines         100632   100656      +24     
  Branches       12995    12996       +1     
=============================================
- Hits           48369    48340      -29     
- Misses         46344    46369      +25     
- Partials        5919     5947      +28

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

RockteMQ-AI

Review: Approved ✅

PR: #10487 — Add getMessageType(Map) overload to eliminate redundant properties decode
Type: Performance (1 file, +4/-1)

Assessment

Eliminates redundant MessageDecoder.string2messageProperties() call in the send hot path. Adds a getMessageType(Map) overload so callers can pass the already-decoded properties map.

Verdict

✅ Clean, minimal change. Fixes #10486.

🤖 Automated review by oss-sentinel-ai

RockteMQ-AI

Review: Approved ✅

PR: #10487 — Add getMessageType(Map) overload to eliminate redundant properties decode
Type: Performance (1 file, +4/-1)

Assessment

Eliminates redundant MessageDecoder.string2messageProperties() call in the send hot path. Adds a getMessageType(Map) overload so callers can pass the already-decoded properties map.

Verdict

✅ Clean, minimal change. Fixes #10486.

🤖 Automated review by oss-sentinel-ai

RockteMQ-AI

Review: Approved ✅

PR: #10487 — Add getMessageType(Map) overload to eliminate redundant properties decode
Type: Performance (1 file, +4/-1)

Assessment

Eliminates redundant MessageDecoder.string2messageProperties() call in the send hot path. Adds a getMessageType(Map) overload so callers can pass the already-decoded properties map.

Verdict

✅ Clean, minimal change. Fixes #10486.

🤖 Automated review by oss-sentinel-ai

RockteMQ-AI

Review: Approved ✅

PR: #10487 — Add getMessageType(Map) overload to eliminate redundant properties decode
Type: Performance (1 file, +4/-1)

Assessment

Eliminates redundant MessageDecoder.string2messageProperties() call in the send hot path. Adds a getMessageType(Map) overload so callers can pass the already-decoded properties map.

Verdict

✅ Clean, minimal change. Fixes #10486.

🤖 Automated review by oss-sentinel-ai

RockteMQ-AI

Review: Approved ✅

PR: #10487 — Add getMessageType(Map) overload to eliminate redundant properties decode
Type: Performance (1 file, +4/-1)

Assessment

Eliminates redundant MessageDecoder.string2messageProperties() call in the send hot path. Adds a getMessageType(Map) overload so callers can pass the already-decoded properties map.

Verdict

✅ Clean, minimal change. Fixes #10486.

🤖 Automated review by oss-sentinel-ai

RockteMQ-AI

Review: Approved ✅

PR: #10487 — Add getMessageType(Map) overload to eliminate redundant properties decode
Type: Performance (1 file, +4/-1)

Assessment

Eliminates redundant MessageDecoder.string2messageProperties() call in the send hot path. Adds a getMessageType(Map) overload so callers can pass the already-decoded properties map.

Verdict

✅ Clean, minimal change. Fixes #10486.

🤖 Automated review by oss-sentinel-ai

RockteMQ-AI

Review: Approved ✅

PR: #10487 — Add getMessageType(Map) overload to eliminate redundant properties decode
Type: Performance (1 file, +4/-1)

Assessment

Eliminates redundant MessageDecoder.string2messageProperties() call in the send hot path. Adds a getMessageType(Map) overload so callers can pass the already-decoded properties map.

Verdict

✅ Clean, minimal change. Fixes #10486.

🤖 Automated review by oss-sentinel-ai

RockteMQ-AI

Review: Approved ✅

PR: #10487 — Add getMessageType(Map) overload to eliminate redundant properties decode
Type: Performance (1 file, +4/-1)

Assessment

Eliminates redundant MessageDecoder.string2messageProperties() call in the send hot path. Adds a getMessageType(Map) overload so callers can pass the already-decoded properties map.

Verdict

✅ Clean, minimal change. Fixes #10486.

🤖 Automated review by oss-sentinel-ai

Copilot AI review requested due to automatic review settings June 11, 2026 09:42

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Comment thread broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java

Comment thread broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java

RockteMQ-AI approved these changes Jun 11, 2026

View reviewed changes

RongtongJin approved these changes Jun 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ISSUE #10486] Add getMessageType(Map) overload to eliminate redundant properties decode#10487

[ISSUE #10486] Add getMessageType(Map) overload to eliminate redundant properties decode#10487
wang-jiahua wants to merge 1 commit into
apache:developfrom
wang-jiahua:perf/metrics-getmessagetype-map-overload

wang-jiahua commented Jun 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jun 11, 2026

Uh oh!

RockteMQ-AI left a comment

Uh oh!

RockteMQ-AI left a comment

Uh oh!

RockteMQ-AI left a comment

Uh oh!

RockteMQ-AI left a comment

Uh oh!

RockteMQ-AI left a comment

Uh oh!

RockteMQ-AI left a comment

Uh oh!

RockteMQ-AI left a comment

Uh oh!

RockteMQ-AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

wang-jiahua commented Jun 11, 2026

Which Issue(s) This PR Fixes

Brief Description

How Did You Test This Change?

Independence

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jun 11, 2026

Codecov Report

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review: Approved ✅

Assessment

Verdict

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review: Approved ✅

Assessment

Verdict

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review: Approved ✅

Assessment

Verdict

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review: Approved ✅

Assessment

Verdict

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review: Approved ✅

Assessment

Verdict

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review: Approved ✅

Assessment

Verdict

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review: Approved ✅

Assessment

Verdict

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review: Approved ✅

Assessment

Verdict

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants