[ 
https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15706174#comment-15706174
 ] 

ASF GitHub Bot commented on GEODE-2141:
---------------------------------------

Github user upthewaterspout commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/299#discussion_r90077733
  
    --- Diff: 
geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java
 ---
    @@ -50,36 +50,26 @@ public StatMonitorHandler() {
     
       /** Adds a monitor which will be notified of samples */
       public boolean addMonitor(StatisticsMonitor monitor) {
    -    synchronized (this) {
    -      boolean added = false;
    -      List<StatisticsMonitor> oldMonitors = this.monitors;
    -      if (!oldMonitors.contains(monitor)) {
    -        List<StatisticsMonitor> newMonitors = new 
ArrayList<StatisticsMonitor>(oldMonitors);
    -        added = newMonitors.add(monitor);
    -        this.monitors = Collections.unmodifiableList(newMonitors);
    -      }
    -      if (!this.monitors.isEmpty()) {
    -        startNotifier_IfEnabledAndNotRunning();
    -      }
    -      return added;
    +    boolean added = false;
    --- End diff --
    
    I don't think it's safe to remove the synchronization here. You're calling 
contains followed by add followed by isEmpty. Also, startNotifier_... is 
dependent on the state of this.monitors. this.monitors could be getting changed 
by other threads at any point in here.


> StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to 
> store monitors, listeners and statisticIds
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: GEODE-2141
>                 URL: https://issues.apache.org/jira/browse/GEODE-2141
>             Project: Geode
>          Issue Type: Improvement
>            Reporter: Avinash Dongre
>            Assignee: Avinash Dongre
>
> In StatisticsMonitor and StatMonitorHandler List is used to store monitors, 
> listeners and statisticIds.
> We should be using ConcurrentHashSet for performance Reason.
> In my local testing  When I replace the List to 
> com.gemstone.gemfire.internal.concurrent.
> ConcurrentHashSet
> I see significant improvement in creating large number of region creation.
> ( from ~7hrs to ~26 minutes)
> More details about the same is on dev list.
> Refer : http://markmail.org/message/o7td3fczylx4uaet



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to