-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131357
-----------------------------------------------------------




geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 1699)
<https://reviews.apache.org/r/46896/#comment195284>

    I don't understand why this needs to be called here as well as in the while 
block. You could initialize it here,but I don't think we need the extra (new 
HashSet()) call here as well. This call is not required.



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 1703)
<https://reviews.apache.org/r/46896/#comment195283>

    This can potentially cause a huge amount of unnecessary garbage. Each time 
this method is called we create a new HashSet.



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 1741)
<https://reviews.apache.org/r/46896/#comment195281>

    I don't think we need synchronized here. There is no chance for any 
concurrent modification here. Even with mutliple threads this is still just a 
getter invocation, there is no danger to concurrently modify anything.



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 1742)
<https://reviews.apache.org/r/46896/#comment195285>

    I don't think we need to explicitly create a new HashSet for a getter. If 
this set needs to be "cloned" then make it the responsibility of the method 
invoking this function to do so.


- Udo Kohlmeyer


On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
> 
> (Updated May 2, 2016, 4:36 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> added synchronization with copyOnread
> 
> 
> Diffs
> -----
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  9f5648b 
> 
> Diff: https://reviews.apache.org/r/46896/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Reply via email to