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