geode-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
Date Tue, 29 Nov 2016 18:50:58 GMT

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

Mime
View raw message