You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "socutes (via GitHub)" <gi...@apache.org> on 2023/02/18 15:23:50 UTC

[GitHub] [rocketmq] socutes opened a new pull request, #6116: [ISSUE #6105] Optimize the timer implementation in Remoting

socutes opened a new pull request, #6116:
URL: https://github.com/apache/rocketmq/pull/6116

   close #6105 


-- 
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


[GitHub] [rocketmq] hYuang commented on a diff in pull request #6116: [ISSUE #6105] Optimize the timer implementation in Remoting

Posted by "hYuang (via GitHub)" <gi...@apache.org>.
hYuang commented on code in PR #6116:
URL: https://github.com/apache/rocketmq/pull/6116#discussion_r1111238766


##########
remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java:
##########
@@ -95,7 +96,7 @@ public class NettyRemotingClient extends NettyRemotingAbstract implements Remoti
     private final ConcurrentHashMap<String /* cidr */, Bootstrap> bootstrapMap = new ConcurrentHashMap<>();
     private final ConcurrentMap<String /* addr */, ChannelWrapper> channelTables = new ConcurrentHashMap<>();
 
-    private final Timer timer = new Timer("ClientHouseKeepingService", true);
+    private final HashedWheelTimer timer = new HashedWheelTimer(r -> new Thread(r, "ClientHouseKeepingService"));

Review Comment:
   we should  keep it simple .  it is necessary ?



-- 
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


[GitHub] [rocketmq] codecov-commenter commented on pull request #6116: [ISSUE #6105] Optimize the timer implementation in Remoting

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6116:
URL: https://github.com/apache/rocketmq/pull/6116#issuecomment-1436223950

   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6116](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dfca035) into [develop](https://codecov.io/gh/apache/rocketmq/commit/975b4f1f820d0a98855ad99707fdcbf68c98ca73?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (975b4f1) will **increase** coverage by `0.03%`.
   > The diff coverage is `81.25%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #6116      +/-   ##
   =============================================
   + Coverage      43.23%   43.27%   +0.03%     
   - Complexity      8846     8856      +10     
   =============================================
     Files           1094     1094              
     Lines          77137    77140       +3     
     Branches       10057    10057              
   =============================================
   + Hits           33350    33379      +29     
   + Misses         39623    39596      -27     
   - Partials        4164     4165       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...e/rocketmq/remoting/netty/NettyRemotingServer.java](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L05ldHR5UmVtb3RpbmdTZXJ2ZXIuamF2YQ==) | `58.41% <80.00%> (-0.86%)` | :arrow_down: |
   | [...e/rocketmq/remoting/netty/NettyRemotingClient.java](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `40.63% <81.81%> (-0.16%)` | :arrow_down: |
   | [...q/namesrv/routeinfo/BrokerHousekeepingService.java](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bmFtZXNydi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbmFtZXNydi9yb3V0ZWluZm8vQnJva2VySG91c2VrZWVwaW5nU2VydmljZS5qYXZh) | `70.00% <0.00%> (-10.00%)` | :arrow_down: |
   | [.../rocketmq/client/producer/RequestFutureHolder.java](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvcHJvZHVjZXIvUmVxdWVzdEZ1dHVyZUhvbGRlci5qYXZh) | `81.39% <0.00%> (-6.98%)` | :arrow_down: |
   | [.../org/apache/rocketmq/remoting/netty/TlsHelper.java](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L1Rsc0hlbHBlci5qYXZh) | `71.57% <0.00%> (-6.32%)` | :arrow_down: |
   | [...org/apache/rocketmq/common/stats/StatsItemSet.java](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvU3RhdHNJdGVtU2V0LmphdmE=) | `44.77% <0.00%> (-5.98%)` | :arrow_down: |
   | [...tmq/remoting/protocol/body/ConsumerConnection.java](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL3Byb3RvY29sL2JvZHkvQ29uc3VtZXJDb25uZWN0aW9uLmphdmE=) | `95.83% <0.00%> (-4.17%)` | :arrow_down: |
   | [...ava/org/apache/rocketmq/filter/util/BitsArray.java](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmlsdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9maWx0ZXIvdXRpbC9CaXRzQXJyYXkuamF2YQ==) | `58.11% <0.00%> (-1.71%)` | :arrow_down: |
   | [...he/rocketmq/client/trace/AsyncTraceDispatcher.java](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvdHJhY2UvQXN5bmNUcmFjZURpc3BhdGNoZXIuamF2YQ==) | `82.21% <0.00%> (-1.45%)` | :arrow_down: |
   | [...rg/apache/rocketmq/remoting/netty/NettyLogger.java](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L05ldHR5TG9nZ2VyLmphdmE=) | `13.60% <0.00%> (-1.37%)` | :arrow_down: |
   | ... and [32 more](https://codecov.io/gh/apache/rocketmq/pull/6116?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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


[GitHub] [rocketmq] drpmma commented on a diff in pull request #6116: [ISSUE #6105] Optimize the timer implementation in Remoting

Posted by "drpmma (via GitHub)" <gi...@apache.org>.
drpmma commented on code in PR #6116:
URL: https://github.com/apache/rocketmq/pull/6116#discussion_r1111385425


##########
remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java:
##########
@@ -95,7 +96,7 @@ public class NettyRemotingClient extends NettyRemotingAbstract implements Remoti
     private final ConcurrentHashMap<String /* cidr */, Bootstrap> bootstrapMap = new ConcurrentHashMap<>();
     private final ConcurrentMap<String /* addr */, ChannelWrapper> channelTables = new ConcurrentHashMap<>();
 
-    private final Timer timer = new Timer("ClientHouseKeepingService", true);
+    private final HashedWheelTimer timer = new HashedWheelTimer(r -> new Thread(r, "ClientHouseKeepingService"));

Review Comment:
   HashedWheelTimer is more accurate than java.util.Time.



-- 
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


[GitHub] [rocketmq] socutes commented on a diff in pull request #6116: [ISSUE #6105] Optimize the timer implementation in Remoting

Posted by "socutes (via GitHub)" <gi...@apache.org>.
socutes commented on code in PR #6116:
URL: https://github.com/apache/rocketmq/pull/6116#discussion_r1111412920


##########
remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java:
##########
@@ -95,7 +96,7 @@ public class NettyRemotingClient extends NettyRemotingAbstract implements Remoti
     private final ConcurrentHashMap<String /* cidr */, Bootstrap> bootstrapMap = new ConcurrentHashMap<>();
     private final ConcurrentMap<String /* addr */, ChannelWrapper> channelTables = new ConcurrentHashMap<>();
 
-    private final Timer timer = new Timer("ClientHouseKeepingService", true);
+    private final HashedWheelTimer timer = new HashedWheelTimer(r -> new Thread(r, "ClientHouseKeepingService"));

Review Comment:
   In my opinion, the network module as the basic component, we should try to optimize every detail.
   Especially in the stream processing scenario, some details of the performance issues, this can cause some unexpected performance problems.
   HashedWheelTimer is more reasonable and better than Timer in terms of performance and implementation.
   



-- 
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


[GitHub] [rocketmq] zhouxinyu merged pull request #6116: [ISSUE #6105] Optimize the timer implementation in Remoting

Posted by "zhouxinyu (via GitHub)" <gi...@apache.org>.
zhouxinyu merged PR #6116:
URL: https://github.com/apache/rocketmq/pull/6116


-- 
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