Skip to content

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

@wang-jiahua

Description

@wang-jiahua

Before Creating the Enhancement Request

  • 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:

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 [ISSUE #10442] Optimize message properties encode/decode to reduce allocation #10444 reduces baseline allocation rate.

Additional Context

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions