You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2021/11/24 08:32:59 UTC

[GitHub] [rocketmq] XiaoyiPeng opened a new pull request #3534: [ISSUE #3533] optimize: It is better to specify the initialCapacity when initializing a HashMap as a local variable that will not escape.

XiaoyiPeng opened a new pull request #3534:
URL: https://github.com/apache/rocketmq/pull/3534


   **Make sure set the target branch to `develop`**
   
   ## What is the purpose of the change
   
   for issue #3533 
   
   ## Brief changelog
   
   Specify the initialCapacity when initializing a `HashMap `as a local variable that will not escape, 
   just like the code in class `org.apache.rocketmq.broker.filter.MessageEvaluationContext#keyValues` :  
   
   ```java
   public Map<String, Object> keyValues() {
           if (properties == null) {
               return null;
           }
   
           Map<String, Object> copy = new HashMap<String, Object>(properties.size(), 1);
   
           for (String key : properties.keySet()) {
               copy.put(key, properties.get(key));
           }
   
           return copy;
       }
   ```
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls commented on pull request #3534: [ISSUE #3533] optimize: It is better to specify the initialCapacity when initializing a HashMap as a local variable that will not add elements in the future.

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3534:
URL: https://github.com/apache/rocketmq/pull/3534#issuecomment-977734029


   
   [![Coverage Status](https://coveralls.io/builds/44509047/badge)](https://coveralls.io/builds/44509047)
   
   Coverage decreased (-0.08%) to 55.014% when pulling **070f172c874f1d9bdec58036677fbb515391eccb on XiaoyiPeng:hashmap_as_a_local_variable_not_escape** into **87b0be00012dff86727766081d4243eeee280699 on apache:develop**.
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] XiaoyiPeng edited a comment on pull request #3534: [ISSUE #3533] optimize: It is better to specify the initialCapacity when initializing a HashMap as a local variable that will not add elements in the future.

Posted by GitBox <gi...@apache.org>.
XiaoyiPeng edited a comment on pull request #3534:
URL: https://github.com/apache/rocketmq/pull/3534#issuecomment-981662904


   > In my opinion, setting loadFactor=1 is not a good way, the default value can satisfy most scenarios.
   
   Thinks for your review.
   
   Here is using the style of the previous version code(like `org.apache.rocketmq.broker.filter.MessageEvaluationContext#keyValues()` ),as shown below:
   ```java
   public Map<String, Object> keyValues() {
           if (properties == null) {
               return null;
           }
   
           Map<String, Object> copy = new HashMap<String, Object>(properties.size(), 1);
   
           for (String key : properties.keySet()) {
               copy.put(key, properties.get(key));
           }
   
           return copy;
       }
   ```
   
   Because we know exactly how many elements this map is going to put in(the map `size` will be small , the probability of **hash conflict** is small ), and we don't want  the map be expansioned(`resize`) in future, I think maybe we can do it like this.
   
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] XiaoyiPeng edited a comment on pull request #3534: [ISSUE #3533] optimize: It is better to specify the initialCapacity when initializing a HashMap as a local variable that will not add elements in the future.

Posted by GitBox <gi...@apache.org>.
XiaoyiPeng edited a comment on pull request #3534:
URL: https://github.com/apache/rocketmq/pull/3534#issuecomment-981662904


   > In my opinion, setting loadFactor=1 is not a good way, the default value can satisfy most scenarios.
   
   Here is using the style of the previous version code(like `org.apache.rocketmq.broker.filter.MessageEvaluationContext#keyValues()` ),as shown below:
   ```java
   public Map<String, Object> keyValues() {
           if (properties == null) {
               return null;
           }
   
           Map<String, Object> copy = new HashMap<String, Object>(properties.size(), 1);
   
           for (String key : properties.keySet()) {
               copy.put(key, properties.get(key));
           }
   
           return copy;
       }
   ```
   
   Because we know exactly how many elements this map is going to put in(the map `size` will be small , the probability of **hash conflict** is small ), and we don't want  the map be expansioned(`resize`) in future, I think maybe we can do it like this.
   
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] XiaoyiPeng closed pull request #3534: [ISSUE #3533] optimize: It is better to specify the initialCapacity when initializing a HashMap as a local variable that will not add elements in the future.

Posted by GitBox <gi...@apache.org>.
XiaoyiPeng closed pull request #3534:
URL: https://github.com/apache/rocketmq/pull/3534


   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] XiaoyiPeng edited a comment on pull request #3534: [ISSUE #3533] optimize: It is better to specify the initialCapacity when initializing a HashMap as a local variable that will not add elements in the future.

Posted by GitBox <gi...@apache.org>.
XiaoyiPeng edited a comment on pull request #3534:
URL: https://github.com/apache/rocketmq/pull/3534#issuecomment-981662904


   > In my opinion, setting loadFactor=1 is not a good way, the default value can satisfy most scenarios.
   
   Here is using the style of the previous version code(like `org.apache.rocketmq.broker.filter.MessageEvaluationContext#keyValues()` ),as shown below:
   ```java
   public Map<String, Object> keyValues() {
           if (properties == null) {
               return null;
           }
   
           Map<String, Object> copy = new HashMap<String, Object>(properties.size(), 1);
   
           for (String key : properties.keySet()) {
               copy.put(key, properties.get(key));
           }
   
           return copy;
       }
   ```
   
   Because we know exactly how many elements this map is going to put in, and we don't want  the map be expansioned(`resize`) in future, I think maybe we can do it like this.
   
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] XiaoyiPeng edited a comment on pull request #3534: [ISSUE #3533] optimize: It is better to specify the initialCapacity when initializing a HashMap as a local variable that will not add elements in the future.

Posted by GitBox <gi...@apache.org>.
XiaoyiPeng edited a comment on pull request #3534:
URL: https://github.com/apache/rocketmq/pull/3534#issuecomment-981662904


   > In my opinion, setting loadFactor=1 is not a good way, the default value can satisfy most scenarios.
   
   Thanks for your review.
   
   Here is using the style of the previous version code(like `org.apache.rocketmq.broker.filter.MessageEvaluationContext#keyValues()` ),as shown below:
   ```java
   public Map<String, Object> keyValues() {
           if (properties == null) {
               return null;
           }
   
           Map<String, Object> copy = new HashMap<String, Object>(properties.size(), 1);
   
           for (String key : properties.keySet()) {
               copy.put(key, properties.get(key));
           }
   
           return copy;
       }
   ```
   
   Because we know exactly how many elements this `map` is going to put in(the map `size` will be small , the probability of **hash conflict** is small ), and we don't want  the `map` be expansioned(`resize`) in future, I think maybe we can do it like this.
   
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] XiaoyiPeng edited a comment on pull request #3534: [ISSUE #3533] optimize: It is better to specify the initialCapacity when initializing a HashMap as a local variable that will not add elements in the future.

Posted by GitBox <gi...@apache.org>.
XiaoyiPeng edited a comment on pull request #3534:
URL: https://github.com/apache/rocketmq/pull/3534#issuecomment-981662904


   > In my opinion, setting loadFactor=1 is not a good way, the default value can satisfy most scenarios.
   
   Thanks for your review.
   
   Here is using the style of the previous version code(like `org.apache.rocketmq.broker.filter.MessageEvaluationContext#keyValues()` ),as shown below:
   ```java
   public Map<String, Object> keyValues() {
           if (properties == null) {
               return null;
           }
   
           Map<String, Object> copy = new HashMap<String, Object>(properties.size(), 1);
   
           for (String key : properties.keySet()) {
               copy.put(key, properties.get(key));
           }
   
           return copy;
       }
   ```
   
   Because we know exactly how many elements this map is going to put in(the map `size` will be small , the probability of **hash conflict** is small ), and we don't want  the map be expansioned(`resize`) in future, I think maybe we can do it like this.
   
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] XiaoyiPeng commented on pull request #3534: [ISSUE #3533] optimize: It is better to specify the initialCapacity when initializing a HashMap as a local variable that will not add elements in the future.

Posted by GitBox <gi...@apache.org>.
XiaoyiPeng commented on pull request #3534:
URL: https://github.com/apache/rocketmq/pull/3534#issuecomment-981662904


   > In my opinion, setting loadFactor=1 is not a good way, the default value can satisfy most scenarios.
   
   Here is using the style of the previous version code(like `org.apache.rocketmq.broker.filter.MessageEvaluationContext#keyValues()` ),as shown below:
   ```java
   public Map<String, Object> keyValues() {
           if (properties == null) {
               return null;
           }
   
           Map<String, Object> copy = new HashMap<String, Object>(properties.size(), 1);
   
           for (String key : properties.keySet()) {
               copy.put(key, properties.get(key));
           }
   
           return copy;
       }
   ```
   
    because we know exactly how many elements this map is going to put in, and we don't want  the map be expansioned(`resize`).
   
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org