qianye1001 opened a new issue, #10400:
URL: https://github.com/apache/rocketmq/issues/10400
### Before Creating the Enhancement Request
- [x] I have confirmed that this should be classified as an enhancement
rather than a bug/feature.
### Summary
`MQVersion.getVersionDesc(int)` and `MQVersion.value2Version(int)` call
`Version.values()` repeatedly (6 times total, 2-3 times per method invocation)
without caching the result. Since Java's `Enum.values()` returns a **new
defensive copy** of the internal array on every call, and the `Version` enum
currently contains **450+ constants**, each invocation allocates and populates
a large array unnecessarily.
### Motivation
- **Unnecessary allocation on hot paths**: These two methods are invoked
from broker metrics reporting (`BrokerMetricsManager`), admin processors,
client factories, proxy pipelines, namesrv request processing, and more —
roughly **21 call sites** across the codebase (excluding tests). In
high-throughput scenarios, the repeated array copying adds up.
- **Well-known Java enum anti-pattern**: Calling `values()` without caching
is a widely recognized performance pitfall in Java. Many projects (including
the JDK itself) cache the result in a `private static final` field.
- **Easy win**: The fix is trivial and risk-free — a one-line static field
addition — but the impact is meaningful given the enum size and call frequency.
### Describe the Solution You'd Like
Add a cached static array and rewrite the two methods to use it:
```java
public class MQVersion {
public static final int CURRENT_VERSION = Version.V5_5_0.ordinal();
private static final Version[] VERSION_VALUES = Version.values();
public static String getVersionDesc(int value) {
if (value >= VERSION_VALUES.length) {
return VERSION_VALUES[VERSION_VALUES.length - 1].name();
}
return VERSION_VALUES[value].name();
}
public static Version value2Version(int value) {
if (value >= VERSION_VALUES.length) {
return VERSION_VALUES[VERSION_VALUES.length - 1];
}
return VERSION_VALUES[value];
}
// ... enum Version unchanged ...
}
```
This eliminates all 6 `values()` calls, replacing them with a single static
initialization.
### Describe Alternatives You've Considered
1. **Cache inside each method with a local variable** — reduces allocations
from 2-3 per call to 1 per call, but still allocates on every invocation. A
static field is strictly better since the enum is immutable.
2. **Use an `EnumMap` or ordinal-indexed `Map`** — over-engineered for this
use case. Direct array indexing by ordinal is already the natural and most
efficient approach, which is exactly what `values()` returns.
### Additional Context
File: `common/src/main/java/org/apache/rocketmq/common/MQVersion.java`
The same pattern (`MessageVersion.values()` without caching) also exists in
`common/src/main/java/org/apache/rocketmq/common/message/MessageVersion.java:74`,
though with a much smaller enum. It may be worth addressing both in the same
PR.
--
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]
