[Enhancement] Eliminate per-RPC allocation in RemotingCommand (Guava Stopwatch, Constructor copy) and downgrade Netty writability log
- 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
Three independent per-RPC allocations in the remoting framework:
1. **Guava `Stopwatch`**: `RemotingCommand` uses `Stopwatch.createStarted()` which allocates a new object per RPC. Replace with `System.nanoTime()` (primitive long).
2. **`Class.getDeclaredConstructor()`**: Copies the `Constructor` object on every call. Cache in a `ConcurrentHashMap, Constructor>` to pay the reflective lookup once per class.
3. **`NettyRemotingServer.channelWritabilityChanged`**: Logs at INFO/WARN on every writability change, triggering `RemotingHelper.parseChannelRemoteAddr()` + String concatenation per event. Downgrade to DEBUG with `isDebugEnabled()` guard.
Additionally: `TopicQueueMappingContext.EMPTY` static singleton to avoid creating empty context objects for non-static-topic messages (>99% of traffic), and `NettyDecoder` adapted to use `setProcessTimerNanos(long)` instead of `setProcessTimer(Stopwatch)`.
### Motivation
JFR `settings=profile` on a broker under steady-state load shows:
- `Class.getDeclaredConstructor()` copies `Constructor` objects — ~237 allocation events per 60s on `RemotingCommand.createResponseCommand` and `decodeCommandCustomHeaderDirectly`
- Guava `Stopwatch.createStarted()` allocates one object per RPC request/response pair
- `NettyRemotingServer.channelWritabilityChanged` logged **81,434 lines** in 90 seconds (~900 lines/sec), each triggering `parseChannelRemoteAddr()` + String concat + AsyncAppender enqueue
### Describe the Solution You'd Like
**1. RemotingCommand — Stopwatch → nanoTime**
```java
// Before
private transient Stopwatch processTimer;
public void markProcessTimer() { processTimer = Stopwatch.createStarted(); }
// After
private transient long processTimerNanos;
public void setProcessTimerNanos(long nanos) { this.processTimerNanos = nanos; }
public long processTimerElapsedMs() { return (System.nanoTime() - processTimerNanos) / 1_000_000; }
```
**2. RemotingCommand — Constructor cache**
```java
private static final Map, Constructor> HEADER_CTOR_CACHE = new ConcurrentHashMap<>();
private static T newHeaderInstance(Class clazz) {
Constructor ctor = HEADER_CTOR_CACHE.computeIfAbsent(clazz, c -> {
try { Constructor ct = c.getDeclaredConstructor(); ct.setAccessible(true); return ct; }
catch (NoSuchMethodException e) { throw new RuntimeException(e); }
});
return (T) ctor.newInstance();
}
```
**3. NettyRemotingServer — log downgrade**
```java
// Before
log.info("Channel[{}] turns writable...", RemotingHelper.parseChannelRemoteAddr(channel), ...);
// After
if (log.isDebugEnabled()) {
log.debug("Channel[{}] turns writable...", RemotingHelper.parseChannelRemoteAddr(channel), ...);
}
```
**4. TopicQueueMappingContext — EMPTY singleton**
```java
public static final TopicQueueMappingContext EMPTY =
new TopicQueueMappingContext(null, null, null, null, null);
```
### Describe Alternatives You've Considered
- **Keeping Stopwatch**: Guava Stopwatch is convenient but allocates on every `createStarted()`. `System.nanoTime()` is JDK built-in and zero-allocation.
- **Reflection caching via MethodHandles**: More complex setup for the same end result. `ConcurrentHashMap` with `computeIfAbsent` is simpler and well-understood.
### Additional Context
Files changed:
- `RemotingCommand.java` — Stopwatch removal + Constructor cache
- `TopicQueueMappingContext.java` — EMPTY singleton
- `NettyRemotingServer.java` — log downgrade
- `NettyDecoder.java` — adapted to `setProcessTimerNanos(long)`
Contributor guide
Research direction
Start with RemotingCommand.java, then trace the related calls in NettyDecoder.java and TopicQueueMappingContext.java; inspect NettyRemotingServer.java for the writability logging path. Done means the listed per-RPC allocations and repeated log-message work are removed while timer propagation, header construction, and empty-context handling still work as described.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, performance
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100