[Enhancement] Reduce allocations in hot-path request headers' toString() by replacing Guava MoreObjects.toStringHelper
- Dominant language
- Java
- Stars
- 22.6k
- Forks
- 12k
- Avg merge
- 3d 1h
- Merged PRs (30d)
- 27
Description
### 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`.
Contributor guide
Research direction
Start with the 14 named request-header classes under remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/, using SendMessageRequestHeader as the initial entry point. Review the existing toStringHelper output and run the differential verification described in the issue, followed by mvn -pl remoting test and the validate phase. Done means byte-for-byte identical output, including null omission and conditional fields, with the stated remoting tests and checkstyle passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- api, backend, performance
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100