You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "wang-jiahua (via GitHub)" <gi...@apache.org> on 2026/07/02 04:59:42 UTC
Re: [PR] [ISSUE #10512] Eliminate per-RPC allocation in RemotingCommand (Guava Stopwatch, Constructor copy) and downgrade Netty writability log [rocketmq]
wang-jiahua commented on code in PR #10514:
URL: https://github.com/apache/rocketmq/pull/10514#discussion_r3510568003
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java:
##########
@@ -509,6 +529,29 @@ public ByteBuffer encodeHeader(final int bodyLength) {
return result;
}
+ public ByteBuffer fastEncodeHeaderAsBuffer(final int bodyLength) {
Review Comment:
Removed from this PR. The `fastEncodeHeaderAsBuffer` method was not wired into any transfer path, so it has been left out to avoid adding unused public surface.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java:
##########
@@ -635,12 +681,17 @@ public void setSerializeTypeCurrentRPC(SerializeType serializeTypeCurrentRPC) {
this.serializeTypeCurrentRPC = serializeTypeCurrentRPC;
}
- public Stopwatch getProcessTimer() {
- return processTimer;
+ public long processTimerElapsedMs() {
Review Comment:
Fixed — `processTimerElapsedMs()` now guards against `processTimerNanos == 0` and returns 0 in that case, so unset commands don't report JVM uptime as request latency.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java:
##########
@@ -635,12 +681,17 @@ public void setSerializeTypeCurrentRPC(SerializeType serializeTypeCurrentRPC) {
this.serializeTypeCurrentRPC = serializeTypeCurrentRPC;
}
- public Stopwatch getProcessTimer() {
- return processTimer;
+ public long processTimerElapsedMs() {
+ return (System.nanoTime() - processTimerNanos) / 1_000_000;
+ }
+
+ @Deprecated
+ public com.google.common.base.Stopwatch getProcessTimer() {
+ return com.google.common.base.Stopwatch.createStarted();
Review Comment:
Added explicit `@deprecated` Javadoc documenting the semantic break: this method returns a newly-started Stopwatch for source compatibility only and does NOT represent the original request processing timer. All internal callers have been migrated to `processTimerElapsedMs()`.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/statictopic/TopicQueueMappingContext.java:
##########
@@ -20,6 +20,8 @@
import java.util.List;
public class TopicQueueMappingContext {
+ public static final TopicQueueMappingContext EMPTY = new TopicQueueMappingContext(null, null, null, null, null);
Review Comment:
Removed from this PR. The `TopicQueueMappingContext.EMPTY` singleton was removed since the class is not truly immutable (has setters).
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java:
##########
@@ -635,12 +658,20 @@ public void setSerializeTypeCurrentRPC(SerializeType serializeTypeCurrentRPC) {
this.serializeTypeCurrentRPC = serializeTypeCurrentRPC;
}
- public Stopwatch getProcessTimer() {
- return processTimer;
+ public long processTimerElapsedMs() {
+ if (processTimerNanos == 0) {
+ return 0;
+ }
+ return (System.nanoTime() - processTimerNanos) / 1_000_000;
+ }
+
+ @Deprecated
+ public com.google.common.base.Stopwatch getProcessTimer() {
Review Comment:
Same as above — added explicit `@deprecated` Javadoc on `getProcessTimer()` documenting the semantic break. The `setProcessTimer(Stopwatch)` method is also kept and deprecated, backed by `processTimerNanos`.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingServer.java:
##########
@@ -581,13 +581,17 @@ public void channelWritabilityChanged(ChannelHandlerContext ctx) throws Exceptio
if (channel.isWritable()) {
if (!channel.config().isAutoRead()) {
channel.config().setAutoRead(true);
- log.info("Channel[{}] turns writable, bytes to buffer before changing channel to un-writable: {}",
- RemotingHelper.parseChannelRemoteAddr(channel), channel.bytesBeforeUnwritable());
+ if (log.isDebugEnabled()) {
+ log.debug("Channel[{}] turns writable, bytes to buffer before changing channel to un-writable: {}",
+ RemotingHelper.parseChannelRemoteAddr(channel), channel.bytesBeforeUnwritable());
+ }
}
} else {
channel.config().setAutoRead(false);
- log.warn("Channel[{}] auto-read is disabled, bytes to drain before it turns writable: {}",
- RemotingHelper.parseChannelRemoteAddr(channel), channel.bytesBeforeWritable());
+ if (log.isDebugEnabled()) {
+ log.debug("Channel[{}] auto-read is disabled, bytes to drain before it turns writable: {}",
Review Comment:
Reverted to original log levels: `log.warn` for auto-read disabled (backpressure), `log.info` for auto-read re-enabled. The downgrade to debug was removed.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org