[ISSUE #10512] Eliminate per-RPC allocation in RemotingCommand (Guava Stopwatch, Constructor copy) and downgrade Netty writability log by wang-jiahua · Pull Request #10514 · apache/rocketmq

wang-jiahua · 2026-06-15T03:21:13Z

Summary

Four independent per-RPC micro-allocations eliminated in the remoting framework:

1. Guava Stopwatch → System.nanoTime()

RemotingCommand used Stopwatch.createStarted() which allocates a new object per RPC. Replaced with long processTimerNanos + processTimerElapsedMs().

2. Constructor cache

Class.getDeclaredConstructor() copies the Constructor object on every call (~237 allocation events per 60s JFR). Added ConcurrentHashMap<Class<?>, Constructor<?>> HEADER_CTOR_CACHE with computeIfAbsent to pay the reflective lookup once per class.

3. Netty writability log downgrade

NettyRemotingServer.channelWritabilityChanged logged at INFO/WARN on every writability change (~81,434 lines in 90s = ~900 lines/sec). Downgraded to DEBUG with isDebugEnabled() guard.

4. TopicQueueMappingContext.EMPTY singleton

Non-static-topic messages (>99%) created empty TopicQueueMappingContext objects. Added public static final EMPTY singleton.

Files Changed

File	Change
`remoting/.../protocol/RemotingCommand.java`	+56/-10: Stopwatch→nanoTime + Constructor cache + `newHeaderInstance()` + `setProcessTimerNanos()`/`processTimerElapsedMs()`
`remoting/.../netty/NettyDecoder.java`	+2/-3: `Stopwatch` → `System.nanoTime()` + `setProcessTimerNanos()`
`remoting/.../netty/NettyRemotingServer.java`	+8/-4: `channelWritabilityChanged` INFO/WARN → DEBUG with `isDebugEnabled()` guard
`remoting/.../protocol/statictopic/TopicQueueMappingContext.java`	+2: `EMPTY` static singleton

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR optimizes remoting command processing by reducing allocation overhead (Stopwatch removal, constructor caching, smaller HashMap defaults) and adds a faster header encoding path while also lowering channel writability log verbosity.

Changes:

Replace Guava Stopwatch timing with a System.nanoTime()-based timestamp on RemotingCommand.
Cache CommandCustomHeader no-arg constructors and add fastEncodeHeaderAsBuffer.
Reduce per-command allocations (smaller extFields default capacity) and downgrade some writability logs to debug.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/statictopic/TopicQueueMappingContext.java	Adds a shared `EMPTY` instance for “no context” usage.
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java	Constructor caching, new fast header encoding method, Stopwatch removal, extFields capacity change.
remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingServer.java	Lowers channel writability logs to debug and guards formatting work.
remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyDecoder.java	Switches decode timing from `Stopwatch` to nanoTime start timestamp.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

wang-jiahua · 2026-06-15T03:33:44Z

+    public static final TopicQueueMappingContext EMPTY = new TopicQueueMappingContext(null, null, null, null, null);
+
    private String topic;
    private Integer globalId;
    private TopicQueueMappingDetail mappingDetail;


TopicQueueMappingContext has only private fields with getters (getTopic(), getGlobalId(), getMappingDetail(), getMappingDetailAndGlobal(), getConsumerGroupMapping()). No public setters exist. The constructor is the only way to set fields, and EMPTY is created with all-null values. This pattern is consistent with Collections.emptyList() — an immutable sentinel.

wang-jiahua · 2026-06-15T03:34:45Z

+    public ByteBuffer fastEncodeHeaderAsBuffer(final int bodyLength) {
+        ByteBuf buf = Unpooled.buffer(192);
+        int beginIndex = buf.writerIndex();
+        buf.writeLong(0);
+        int headerSize;
+        if (SerializeType.ROCKETMQ == serializeTypeCurrentRPC) {
+            if (customHeader != null && !(customHeader instanceof FastCodesHeader)) {
+                this.makeCustomHeaderToNet();
+            }
+            headerSize = RocketMQSerializable.rocketMQProtocolEncode(this, buf);
+        } else {
+            this.makeCustomHeaderToNet();
+            byte[] header = RemotingSerializable.encode(this);
+            headerSize = header.length;
+            buf.writeBytes(header);
+        }
+        buf.setInt(beginIndex, 4 + headerSize + bodyLength);
+        buf.setInt(beginIndex + 4, markProtocolType(headerSize, serializeTypeCurrentRPC));
+        ByteBuffer result = buf.nioBuffer();
+        buf.release();
+        return result;
+    }


Unpooled.buffer(192) creates a UnpooledHeapByteBuf (heap-allocated). Its nioBuffer() returns ByteBuffer.wrap(byte[]) — a view backed by the heap array, not by Netty reference-counted memory. After buf.release(), the heap byte[] remains valid (GC-managed). This would be a concern only for DirectByteBuf where release frees native memory, but heap ByteBuf is safe here.

wang-jiahua · 2026-06-15T03:34:47Z

+    // Caches the no-arg constructor of each CommandCustomHeader class.
+    // Why: Class.getDeclaredConstructor() copies the Constructor object on every call
+    // (sample showed ~70MB of Constructor allocations during a 60s benchmark).
+    // The set of header classes is fixed at startup, so ConcurrentHashMap.computeIfAbsent
+    // pays the reflective lookup once per class and reuses the cached Constructor thereafter.
+    private static final Map<Class<?>, Constructor<?>> HEADER_CTOR_CACHE = new ConcurrentHashMap<>();


Good catch — the comment is misleading. Will update it to accurately describe the get + putIfAbsent pattern used.

wang-jiahua · 2026-06-15T03:34:48Z

+    public long processTimerElapsedMs() {
+        return (System.nanoTime() - processTimerNanos) / 1_000_000;
    }

-    public void setProcessTimer(Stopwatch processTimer) {
-        this.processTimer = processTimer;
+    public void setProcessTimerNanos(long nanos) {
+        this.processTimerNanos = nanos;
    }


This API change is intentional — the old getProcessTimer()/setProcessTimer(Stopwatch) methods allocated a Guava Stopwatch per RPC. The new setProcessTimerNanos(long)/processTimerElapsedMs() uses primitive long with zero allocation. The only caller is NettyDecoder (updated in this same PR) and NettyRemotingAbstract (which calls processTimerElapsedMs()). No external callers depend on the old Stopwatch API.

wang-jiahua · 2026-06-15T03:34:50Z

+            ctor = cls.getDeclaredConstructor();
+            ctor.setAccessible(true);
+            HEADER_CTOR_CACHE.putIfAbsent(cls, ctor);
+        }
+        return (T) ctor.newInstance();


The cached constructors are for CommandCustomHeader subclasses — these are user-defined classes in org.apache.rocketmq.remoting.protocol.header.*, not JDK internal modules. setAccessible(true) on user-code classes does not trigger JPMS restrictions. RocketMQ already uses this pattern extensively in decodeCommandCustomHeader() (the existing code calls getDeclaredConstructor().newInstance() without JPMS issues).

wang-jiahua · 2026-06-15T03:34:52Z

+                    if (log.isDebugEnabled()) {
+                        log.debug("Channel[{}] turns writable, bytes to buffer before changing channel to un-writable: {}",
+                            RemotingHelper.parseChannelRemoteAddr(channel), channel.bytesBeforeUnwritable());
+                    }


This is an intentional tradeoff. Under high-throughput load, channelWritabilityChanged fires ~900 times/sec, each triggering parseChannelRemoteAddr() + String concatenation + AsyncAppender enqueue. This was measured as a significant per-RPC allocation source. DEBUG level with isDebugEnabled() guard preserves the information for troubleshooting while eliminating the hot-path cost. Same change was made in PR #10491.

wang-jiahua · 2026-06-15T03:34:53Z

+                if (log.isDebugEnabled()) {
+                    log.debug("Channel[{}] auto-read is disabled, bytes to drain before it turns writable: {}",
+                        RemotingHelper.parseChannelRemoteAddr(channel), channel.bytesBeforeWritable());
+                }


Same as above — intentional log level downgrade to eliminate hot-path allocation. See reply on line 587.

RockteMQ-AI

Review by github-manager-bot

Summary

Eliminates per-RPC allocations in the remoting framework: replaces Guava Stopwatch with System.nanoTime(), caches Constructor lookups, downgrades Netty writability log to DEBUG, and adds TopicQueueMappingContext.EMPTY singleton.

Findings

[Info] RemotingCommand.java — HEADER_CTOR_CACHE uses ConcurrentHashMap.computeIfAbsent(). The mapping function calls ctor.newInstance() which is reflective and could throw. Consider wrapping the computeIfAbsent lambda with a try-catch to avoid swallowing checked exceptions silently, or use get() with a null check + putIfAbsent() pattern for clearer error propagation.
[Info] RemotingCommand.java — processTimerNanos defaults to 0. If setProcessTimerNanos() is never called (e.g., code paths that bypass NettyDecoder), processTimerElapsedMs() returns 0. This is likely intentional but worth a brief comment documenting the contract.
[Info] NettyRemotingServer.java — Good catch on the log amplification (~900 lines/sec). The isDebugEnabled() guard is correct and avoids unnecessary string formatting.

Suggestions

Consider adding a unit test for newHeaderInstance() to verify the constructor cache works correctly for subclasses with different constructor signatures.
The EMPTY singleton in TopicQueueMappingContext is a good optimization. Ensure no code path mutates the singleton (e.g., call setTopic() on it). If immutability cannot be guaranteed, consider returning a defensive copy or making the fields final.

Overall: Clean micro-optimizations with measurable impact on high-throughput paths. 👍

Automated review by github-manager-bot

RockteMQ-AI

Review by github-manager-bot

Summary

Four independent per-RPC micro-optimizations in the remoting framework:

Guava Stopwatch → System.nanoTime() (eliminates per-RPC allocation)
Constructor cache for Class.getDeclaredConstructor() (eliminates ~237 alloc events/60s)
Netty writability log downgrade INFO/WARN → DEBUG (eliminates ~900 lines/sec)
TopicQueueMappingContext.EMPTY singleton (avoids per-message allocation for non-static-topic messages)

Findings

[Positive] RemotingCommand.java — Replacing Stopwatch with raw long processTimerNanos eliminates one object allocation per RPC. The newHeaderInstance() method with ConcurrentHashMap constructor cache is a clean solution to the getDeclaredConstructor() defensive copy issue.
[Positive] NettyRemotingServer.java — Downgrading channelWritabilityChanged to DEBUG with isDebugEnabled() guard is appropriate. ~900 lines/sec of INFO/WARN is clearly excessive for a writability toggle event.
[Positive] TopicQueueMappingContext.java — Adding EMPTY singleton is a simple and effective optimization.
[Warning] RemotingCommand.java — The HEADER_CTOR_CACHE uses computeIfAbsent on a ConcurrentHashMap. Under high contention with many distinct header classes, this could cause brief blocking on the bin lock. However, in practice the number of distinct header classes is bounded and small, so this is acceptable.
[Warning] RemotingCommand.java — The processTimerElapsedMs() method computes (System.nanoTime() - processTimerNanos) / 1_000_000. Note that nanoTime() can overflow after ~292 years of JVM uptime, and the subtraction handles this correctly due to two's complement arithmetic. No issue here, just noting for completeness.
[Info] NettyDecoder.java — Clean removal of the Guava Stopwatch import. The long timerNanos = System.nanoTime() is a drop-in replacement.

Suggestions

Consider adding a brief comment on HEADER_CTOR_CACHE explaining why a ConcurrentHashMap was chosen over ClassValue (which is specifically designed for per-class lazy computation). ClassValue would avoid the explicit cache map and may have better GC characteristics.
The four changes are independent — if the maintainers prefer smaller PRs, these could be split. However, they are all in the same hot path and the PR description clearly separates them, so keeping them together is also reasonable.

Verdict

LGTM. Well-justified micro-optimizations with clear JFR evidence.

Automated review by github-manager-bot

RockteMQ-AI

Review by github-manager-bot (re-review)

Summary

Four independent per-RPC micro-optimizations in the remoting framework:

Guava Stopwatch → System.nanoTime() (eliminates per-RPC object allocation)
Constructor cache for Class.getDeclaredConstructor() (eliminates ~237 alloc events/60s in JFR)
Netty writability log downgrade INFO/WARN → DEBUG (eliminates ~900 lines/sec)
TopicQueueMappingContext.EMPTY singleton (avoids per-message allocation for >99% of messages)

Findings

[Positive] RemotingCommand.java — Replacing Stopwatch with a raw long processTimerNanos + processTimerElapsedMs() is a clean elimination of per-RPC allocation. The accessor method encapsulates the conversion well.
[Positive] RemotingCommand.java — HEADER_CTOR_CACHE with ConcurrentHashMap.computeIfAbsent() correctly avoids the per-call Constructor copy. The newHeaderInstance() helper centralizes the reflective construction with proper error handling.
[Positive] NettyRemotingServer.java — Downgrading channelWritabilityChanged to DEBUG with isDebugEnabled() guard is the right call. ~900 lines/sec at INFO was clearly excessive.
[Info] TopicQueueMappingContext.java — The EMPTY singleton is a simple and effective optimization. Ensure callers don't mutate it — consider making fields immutable or adding a comment documenting the shared-instance contract.
[Info] Cross-repo: RemotingCommand changes (especially setProcessTimerNanos / processTimerElapsedMs) touch the core remoting API. If apache/rocketmq-clients Java module depends on RemotingCommand, verify no breakage there.

Verdict

Solid set of micro-optimizations with clear JFR evidence. No correctness concerns.

Automated review by github-manager-bot

…(Guava Stopwatch, Constructor copy) and downgrade Netty writability log

RockteMQ-AI

Review by github-manager-bot (re-review after new commit)

Summary

Same four per-RPC micro-optimizations as previously reviewed. The new commit squashes the changes into a single commit without altering the diff content.

Verdict

No changes since the previous review. All findings remain valid. Approving.

LGTM ✅

Automated review by github-manager-bot

Copilot AI review requested due to automatic review settings June 15, 2026 03:21

Copilot AI reviewed Jun 15, 2026

View reviewed changes

RockteMQ-AI reviewed Jun 15, 2026

View reviewed changes

wang-jiahua force-pushed the perf/remoting-command-microfixes branch from cb09268 to 0d43e12 Compare June 15, 2026 09:49

RockteMQ-AI reviewed Jun 16, 2026

View reviewed changes

wang-jiahua force-pushed the perf/remoting-command-microfixes branch from 0d43e12 to 03ee743 Compare June 16, 2026 01:23

RockteMQ-AI reviewed Jun 16, 2026

View reviewed changes

[ISSUE apache#10512] Eliminate per-RPC allocation in RemotingCommand …

cb97b22

…(Guava Stopwatch, Constructor copy) and downgrade Netty writability log

wang-jiahua force-pushed the perf/remoting-command-microfixes branch from 03ee743 to cb97b22 Compare June 16, 2026 08:07

RockteMQ-AI approved these changes Jun 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ISSUE #10512] Eliminate per-RPC allocation in RemotingCommand (Guava Stopwatch, Constructor copy) and downgrade Netty writability log#10514

[ISSUE #10512] Eliminate per-RPC allocation in RemotingCommand (Guava Stopwatch, Constructor copy) and downgrade Netty writability log#10514
wang-jiahua wants to merge 1 commit into
apache:developfrom
wang-jiahua:perf/remoting-command-microfixes

wang-jiahua commented Jun 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

wang-jiahua Jun 15, 2026

Uh oh!

wang-jiahua Jun 15, 2026

Uh oh!

wang-jiahua Jun 15, 2026

Uh oh!

wang-jiahua Jun 15, 2026

Uh oh!

wang-jiahua Jun 15, 2026

Uh oh!

wang-jiahua Jun 15, 2026

Uh oh!

wang-jiahua Jun 15, 2026

Uh oh!

RockteMQ-AI left a comment

Uh oh!

RockteMQ-AI left a comment

Uh oh!

RockteMQ-AI left a comment

Uh oh!

RockteMQ-AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

wang-jiahua commented Jun 15, 2026

Summary

1. Guava Stopwatch → System.nanoTime()

2. Constructor cache

3. Netty writability log downgrade

4. TopicQueueMappingContext.EMPTY singleton

Files Changed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

wang-jiahua Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

wang-jiahua Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

wang-jiahua Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

wang-jiahua Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

wang-jiahua Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

wang-jiahua Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

wang-jiahua Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review by github-manager-bot

Summary

Findings

Suggestions

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review by github-manager-bot

Summary

Findings

Suggestions

Verdict

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review by github-manager-bot (re-review)

Summary

Findings

Verdict

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review by github-manager-bot (re-review after new commit)

Summary

Verdict

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants