You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
publicstaticTopicMessageTypegetMessageType(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 MappublicstaticTopicMessageTypegetMessageType(SendMessageRequestHeaderrequestHeader) {
returngetMessageType(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.
Before Creating the Enhancement Request
Summary
BrokerMetricsManager.getMessageType(SendMessageRequestHeader)is called once per send to classify the message (NORMAL/TRANSACTION/DELAY/FIFO). It internally decodes the properties String into aHashMapby callingMessageDecoder.string2messageProperties(...)— but in the typical send path, the callerSendMessageProcessorhas already decoded the same properties String just before calling this method (e.g. inbuildMsgContext). The result is a redundant decode on every send: a freshHashMap(with associatedNode[],Stringsubstrings, 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-decodedMapand reuse it.Motivation
Per-send call chain in
SendMessageProcessor:JFR sampling on broker shows
string2messagePropertiesaccounts 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 oneHashMap+ ~14Stringsubstrings + 1Node[](size 32 after PR #10444's right-sizing, or 128 before).Describe the Solution You'd Like
Add a public overload on
BrokerMetricsManager:The downstream
SendMessageProcessorchange (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
RequestHeader: rejected becauseRemotingCommandis the wire object, not a decode cache; mutating it for this purpose would muddy the layering.Additional Context
AttributeKeyinstances #10441 (parent issue covering the broader metrics allocation reduction).AttributeKeyinstances #10443 / [Enhancement] Cache OTel Attributes objects in broker/remoting metrics hot path #10481 — the new overload doesn't reference any new constants.