qianye1001 opened a new issue, #11003:
URL: https://github.com/apache/rocketmq/issues/11003
### Before Creating the Enhancement Request
- [X] I have confirmed that this should be classified as an enhancement
rather than a bug/feature.
### Summary
Replace Guava `MoreObjects.toStringHelper` with a pre-sized `StringBuilder`
in the `toString()` of the high-frequency request header classes under
`remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/`. The
rendered output stays byte-for-byte identical; the change removes roughly N+3
short-lived allocations and an O(N) linked-list traversal per call for a header
with N fields.
Scope (14 send / consume / ack / offset hot-path headers):
`SendMessageRequestHeader`, `SendMessageRequestHeaderV2`,
`PopMessageRequestHeader`, `PopLiteMessageRequestHeader`,
`PullMessageRequestHeader`, `NotificationRequestHeader`,
`AckMessageRequestHeader`, `ChangeInvisibleTimeRequestHeader`,
`QueryConsumerOffsetRequestHeader`, `UpdateConsumerOffsetRequestHeader`,
`GetMaxOffsetRequestHeader`, `GetMinOffsetRequestHeader`,
`ConsumerSendMsgBackRequestHeader`, `RecallMessageRequestHeader`.
Measured on the module's current build target (JDK 8 / `target 1.8`):
**1.62x - 2.60x faster `toString()` and 32% - 48% less allocation per call**.
### Motivation
These headers render themselves with `MoreObjects.toStringHelper`, e.g.
`SendMessageRequestHeader`:
```java
return MoreObjects.toStringHelper(this)
.add("producerGroup", producerGroup)
.add("topic", topic)
... // 13 fields in total
.toString();
```
Per invocation this allocates, before the resulting `String` is even built:
1. one `ToStringHelper` object;
2. one `ValueHolder` head plus **one `ValueHolder` node per `.add(...)`**,
forming a singly linked list;
3. `getClass().getSimpleName()`, which itself allocates (`getName()` +
`substring`);
4. an internal `StringBuilder(32)` that grows through repeated
`Arrays.copyOf` whenever the rendering exceeds 32 characters;
and then traverses that linked list to render. For a header with N fields
this is about **N + 3 short-lived objects plus an O(N) traversal per call**.
`toString()` on these headers sits on hot paths: it is invoked whenever a
header is rendered for logging or diagnostics (request logging, exception
messages, troubleshooting output). At high message throughput this becomes a
continuous stream of short-lived garbage and a measurable amount of CPU spent
purely on rendering, which also shows up as extra young-generation collection
work.
### Describe the Solution You'd Like
Use a **pre-sized `StringBuilder`** with direct `append` calls:
- **Plain headers** - a single chained `append` sequence. The initial
capacity is derived from the class's fixed skeleton (class name + field names +
`=` / `, ` separators) plus an allowance for variable-length values such as
`properties`, `subscription` and `extraInfo`. Pre-sizing matters here: a
default-capacity builder needs 5-6 growth-and-copy steps to reach a ~350
character rendering.
- **Headers using `.omitNullValues()`** (`AckMessageRequestHeader`,
`ChangeInvisibleTimeRequestHeader`, `NotificationRequestHeader`) - conditional
appends that skip null values, preserving the exact omission semantics.
Primitive-typed fields are always emitted, which matches today's behaviour
since they autobox to a non-null value.
- **Conditional entries** such as `.add("isLiteConsumer", isLiteConsumer ?
true : null)` in `NotificationRequestHeader` are preserved by evaluating the
expression into a local and null-checking it, so the entry is still rendered
only when `true`.
The rendered strings must remain **byte-for-byte identical**, because they
end up in logs that people and external tooling parse.
### Describe Alternatives You've Considered
**1. Plain `+` string concatenation.** More concise, and on a JDK 9+
bytecode target it compiles to `invokedynamic` /
`StringConcatFactory.makeConcatWithConstants`, which beats anything
hand-written - measured **4.53x** and **-73%** allocation on JDK 21.
However this module builds with `maven.compiler.source/target = 1.8`, where
`javac` lowers `+` to `new StringBuilder()` with the default capacity of 16.
For headers whose rendering runs to a few hundred characters (those carrying
`properties` / `subscription` / `extraInfo`) that means 5-6 growth-and-copy
steps, which cancels out most of the benefit of dropping Guava. Measured for
`SendMessageRequestHeader` at `target 1.8`:
| form | alloc/call | throughput vs Guava |
|---|---|---|
| `+` concatenation | 3664 B (-2.6%) | 1.27x |
| pre-sized `StringBuilder` | 1944 B (-48.3%) | **2.07x** |
So plain concatenation was rejected for now: it is the worse option under
the project's current build target. The pre-sized builder also stays good on
newer targets (1.95x on JDK 21), so it does not become a liability. **If the
project later raises the bytecode target, switching these to plain `+`
concatenation would be a worthwhile follow-up.**
**2. Removing or gating the `toString()` calls at the call sites.** Out of
scope: it changes observable logging behaviour rather than making the existing
rendering cheaper.
**3. Also converting the remaining ~10 header classes that use
`toStringHelper`.** Left out deliberately - those are admin / low-frequency
headers, and keeping them out makes this change easier to review. They can
follow up separately.
### Additional Context
#### Measured effect
Microbenchmark against the real classes, JDK 8 runtime, the module's `target
1.8` bytecode, realistic field values (including a ~120 character
`properties`). Allocation measured with `ThreadMXBean.getThreadAllocatedBytes`:
| Header | alloc before | alloc after | reduction | throughput |
|---|---|---|---|---|
| `SendMessageRequestHeader` (13 fields) | 3760 B | 1944 B | **-48.3%** |
**2.07x** |
| `PullMessageRequestHeader` (15 fields) | 4072 B | 2328 B | -42.8% | 1.79x |
| `PopMessageRequestHeader` (12 fields) | 2456 B | 1432 B | -41.7% | 2.08x |
| `AckMessageRequestHeader` (6 fields, omitNullValues) | 1848 B | 1176 B |
-36.4% | 1.80x |
| `ChangeInvisibleTimeRequestHeader` (8 fields, omitNullValues) | 2088 B |
1416 B | -32.2% | 1.62x |
| `QueryConsumerOffsetRequestHeader` (4 fields) | 1104 B | 608 B | -44.9% |
**2.60x** |
#### Correctness verification
A differential test loads the original and the modified compiled classes in
two separate class loaders, applies identical field values to both instances
and compares `toString()`. Value plans cover: all-default, all-null,
all-non-null, **each field individually null**, **each field individually
set**, boolean fields in both states (this is what covers the conditional
`isLiteConsumer` entry), plus 40,000 randomized adversarial value sets per
class (null, empty, `\r\n\t`, other control characters, 300- and 2000-character
strings, non-ASCII, `Integer`/`Long` `MAX_VALUE`/`MIN_VALUE`).
- Differential test: **560,483 cases, 0 mismatches**
- `mvn -pl remoting test`: **174 tests, 0 failures**
- checkstyle (`style/rmq_checkstyle.xml`, `validate` phase): **0 violations**
As a check on the test itself, deleting the conditional `isLiteConsumer`
handling makes the differential test fail immediately and prints the exact
divergence, confirming it can detect this class of regression rather than
passing vacuously.
Verified against `develop` at `bee586bcd`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
