[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15713698#comment-15713698 ] ASF subversion and git services commented on GEODE-2141: Commit 50320dc4e828c900b6e1ee92c909fa019cae26d0 in incubator-geode's branch refs/heads/feature/GEODE-2156 from adongre [ https://git-wip-us.apache.org/repos/asf?p=incubator-geode.git;h=50320dc ] GEODE-2141: Replace use of List with ConcurrentHashSet for storing various stats and listener. Moved methods stopNotifier_IfEnabledAndRunning and startNotifier_IfEnabledAndNotRunning in synchronized block. GEODE-2141: Addresing the review comments. GEODE-2141: Addressing Review Comments Making addMonitor and remoteMonitor threadsafe. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15713700#comment-15713700 ] ASF subversion and git services commented on GEODE-2141: Commit 50320dc4e828c900b6e1ee92c909fa019cae26d0 in incubator-geode's branch refs/heads/feature/GEODE-2156 from adongre [ https://git-wip-us.apache.org/repos/asf?p=incubator-geode.git;h=50320dc ] GEODE-2141: Replace use of List with ConcurrentHashSet for storing various stats and listener. Moved methods stopNotifier_IfEnabledAndRunning and startNotifier_IfEnabledAndNotRunning in synchronized block. GEODE-2141: Addresing the review comments. GEODE-2141: Addressing Review Comments Making addMonitor and remoteMonitor threadsafe. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15713699#comment-15713699 ] ASF subversion and git services commented on GEODE-2141: Commit 50320dc4e828c900b6e1ee92c909fa019cae26d0 in incubator-geode's branch refs/heads/feature/GEODE-2156 from adongre [ https://git-wip-us.apache.org/repos/asf?p=incubator-geode.git;h=50320dc ] GEODE-2141: Replace use of List with ConcurrentHashSet for storing various stats and listener. Moved methods stopNotifier_IfEnabledAndRunning and startNotifier_IfEnabledAndNotRunning in synchronized block. GEODE-2141: Addresing the review comments. GEODE-2141: Addressing Review Comments Making addMonitor and remoteMonitor threadsafe. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15713658#comment-15713658 ] ASF subversion and git services commented on GEODE-2141: Commit 50320dc4e828c900b6e1ee92c909fa019cae26d0 in incubator-geode's branch refs/heads/develop from adongre [ https://git-wip-us.apache.org/repos/asf?p=incubator-geode.git;h=50320dc ] GEODE-2141: Replace use of List with ConcurrentHashSet for storing various stats and listener. Moved methods stopNotifier_IfEnabledAndRunning and startNotifier_IfEnabledAndNotRunning in synchronized block. GEODE-2141: Addresing the review comments. GEODE-2141: Addressing Review Comments Making addMonitor and remoteMonitor threadsafe. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15713657#comment-15713657 ] ASF subversion and git services commented on GEODE-2141: Commit 50320dc4e828c900b6e1ee92c909fa019cae26d0 in incubator-geode's branch refs/heads/develop from adongre [ https://git-wip-us.apache.org/repos/asf?p=incubator-geode.git;h=50320dc ] GEODE-2141: Replace use of List with ConcurrentHashSet for storing various stats and listener. Moved methods stopNotifier_IfEnabledAndRunning and startNotifier_IfEnabledAndNotRunning in synchronized block. GEODE-2141: Addresing the review comments. GEODE-2141: Addressing Review Comments Making addMonitor and remoteMonitor threadsafe. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15713659#comment-15713659 ] ASF subversion and git services commented on GEODE-2141: Commit 50320dc4e828c900b6e1ee92c909fa019cae26d0 in incubator-geode's branch refs/heads/develop from adongre [ https://git-wip-us.apache.org/repos/asf?p=incubator-geode.git;h=50320dc ] GEODE-2141: Replace use of List with ConcurrentHashSet for storing various stats and listener. Moved methods stopNotifier_IfEnabledAndRunning and startNotifier_IfEnabledAndNotRunning in synchronized block. GEODE-2141: Addresing the review comments. GEODE-2141: Addressing Review Comments Making addMonitor and remoteMonitor threadsafe. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15713664#comment-15713664 ] ASF GitHub Bot commented on GEODE-2141: --- Github user davinash closed the pull request at: https://github.com/apache/incubator-geode/pull/299 > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15712635#comment-15712635 ] ASF GitHub Bot commented on GEODE-2141: --- Github user upthewaterspout commented on the issue: https://github.com/apache/incubator-geode/pull/299 +1 - looks good to me, I think it's ready to merge. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15710579#comment-15710579 ] ASF GitHub Bot commented on GEODE-2141: --- Github user davinash commented on the issue: https://github.com/apache/incubator-geode/pull/299 Is this PR good to merge to develop ? > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15708547#comment-15708547 ] ASF GitHub Bot commented on GEODE-2141: --- Github user davinash commented on the issue: https://github.com/apache/incubator-geode/pull/299 Thanks @upthewaterspout , @metatype for review I Have address review comments. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[
https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15706173#comment-15706173
]
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_r90078053
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java
---
@@ -138,8 +127,8 @@ public void allocatedResourceInstance(ResourceInstance
resourceInstance) {}
public void destroyedResourceInstance(ResourceInstance resourceInstance)
{}
/** For testing only */
- List getMonitorsSnapshot() {
-return Collections.unmodifiableList(this.monitors);
+ ConcurrentHashSet getMonitorsSnapshot() {
--- End diff --
Maybe this ought to just return a Collection?
> 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[
https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15706175#comment-15706175
]
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_r90077848
--- 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 oldMonitors = this.monitors;
- if (!oldMonitors.contains(monitor)) {
-List newMonitors = new
ArrayList(oldMonitors);
-added = newMonitors.add(monitor);
-this.monitors = Collections.unmodifiableList(newMonitors);
- }
- if (!this.monitors.isEmpty()) {
-startNotifier_IfEnabledAndNotRunning();
- }
- return added;
+boolean added = false;
+if (!this.monitors.contains(monitor)) {
+ added = this.monitors.add(monitor);
+}
+if (!this.monitors.isEmpty()) {
+ startNotifier_IfEnabledAndNotRunning();
}
+return added;
}
/** Removes a monitor that will no longer be used */
public boolean removeMonitor(StatisticsMonitor monitor) {
-synchronized (this) {
- boolean removed = false;
- List oldMonitors = this.monitors;
- if (oldMonitors.contains(monitor)) {
-List newMonitors = new
ArrayList(oldMonitors);
-removed = newMonitors.remove(monitor);
-this.monitors = Collections.unmodifiableList(newMonitors);
- }
- if (this.monitors.isEmpty()) {
-stopNotifier_IfEnabledAndRunning();
- }
- return removed;
+boolean removed = false;
--- End diff --
Same issue here - it's not safe to remove the synchronization.
> 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[
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 oldMonitors = this.monitors;
- if (!oldMonitors.contains(monitor)) {
-List newMonitors = new
ArrayList(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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15705530#comment-15705530 ] ASF GitHub Bot commented on GEODE-2141: --- Github user metatype commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/299#discussion_r90025277 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java --- @@ -38,7 +37,8 @@ private final boolean enableMonitorThread; /** The registered monitors */ - private volatile List monitors = Collections.emptyList(); + private volatile ConcurrentHashSet monitors = --- End diff -- Instead of `volatile` I think this should be final. I don't see a write to this var after initialization. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[
https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15705535#comment-15705535
]
ASF GitHub Bot commented on GEODE-2141:
---
Github user metatype commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/299#discussion_r90025760
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java
---
@@ -110,7 +100,7 @@ public void sampled(long nanosTimeStamp,
List resourceInstance
}
private void monitor(final long sampleTimeMillis, final
List resourceInstance) {
-List currentMonitors =
StatMonitorHandler.this.monitors;
+ConcurrentHashSet currentMonitors =
StatMonitorHandler.this.monitors;
--- End diff --
I don't think this copy is needed since `monitors` is never updated after
initialization.
> 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15705532#comment-15705532 ] ASF GitHub Bot commented on GEODE-2141: --- Github user metatype commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/299#discussion_r90025338 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java --- @@ -29,23 +29,20 @@ private final Object mutex = new Object(); - private volatile List listeners = Collections.emptyList(); + private volatile ConcurrentHashSet listeners = --- End diff -- Instead of `volatile` I think this should be final. I don't see a write to this var after initialization. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[
https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15705531#comment-15705531
]
ASF GitHub Bot commented on GEODE-2141:
---
Github user metatype commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/299#discussion_r90026482
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java
---
@@ -114,14 +100,14 @@ protected void monitor(long millisTimeStamp,
List resourceInst
private final void monitorStatisticIds(long millisTimeStamp,
List resourceInstances) {
-List statisticIdsToMonitor = statisticIds;
+ConcurrentHashSet statisticIdsToMonitor = statisticIds;
--- End diff --
I don't think this copy is needed since `statisticIdsToMonitor` is never
updated after initialization.
> 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[
https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15705534#comment-15705534
]
ASF GitHub Bot commented on GEODE-2141:
---
Github user metatype commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/299#discussion_r90025856
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java
---
@@ -228,7 +222,7 @@ private void work() {
}
}
if (working && latestTask != null) {
-List currentMonitors =
StatMonitorHandler.this.monitors;
+ConcurrentHashSet currentMonitors =
StatMonitorHandler.this.monitors;
--- End diff --
I don't think this copy is needed since `monitors` is never updated after
initialization.
> 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15705533#comment-15705533 ] ASF GitHub Bot commented on GEODE-2141: --- Github user metatype commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/299#discussion_r90025325 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java --- @@ -29,23 +29,20 @@ private final Object mutex = new Object(); - private volatile List listeners = Collections.emptyList(); + private volatile ConcurrentHashSet listeners = + new ConcurrentHashSet(); - private volatile List statisticIds = Collections.emptyList(); + private volatile ConcurrentHashSet statisticIds = --- End diff -- Instead of `volatile` I think this should be final. I don't see a write to this var after initialization. > 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[
https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15705536#comment-15705536
]
ASF GitHub Bot commented on GEODE-2141:
---
Github user metatype commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/299#discussion_r90025623
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java
---
@@ -114,14 +100,14 @@ protected void monitor(long millisTimeStamp,
List resourceInst
private final void monitorStatisticIds(long millisTimeStamp,
List resourceInstances) {
-List statisticIdsToMonitor = statisticIds;
+ConcurrentHashSet statisticIdsToMonitor = statisticIds;
if (!statisticIdsToMonitor.isEmpty()) {
// TODO:
}
}
protected final void notifyListeners(StatisticsNotification
notification) {
-List listenersToNotify = this.listeners;
+ConcurrentHashSet listenersToNotify =
this.listeners;
--- End diff --
I don't think this copy is needed since `listeners` is never updated after
initialization.
> 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)
[jira] [Commented] (GEODE-2141) StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to store monitors, listeners and statisticIds
[ https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15704812#comment-15704812 ] ASF GitHub Bot commented on GEODE-2141: --- GitHub user davinash opened a pull request: https://github.com/apache/incubator-geode/pull/299 [ GEODE-2141 ] #comment Fix Issue #2141 Replaced List with ConcurrentHashSet You can merge this pull request into a Git repository by running: $ git pull https://github.com/davinash/incubator-geode feature/GEODE-2141 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-geode/pull/299.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #299 commit e4da4c43dc691e59d34640e95c5d63a5a1b0ea1c Author: adongre Date: 2016-11-27T14:11:48Z GEODE-2141: Replace use of List with ConcurrentHashSet for storing various stats and listener. Moved methods stopNotifier_IfEnabledAndRunning and startNotifier_IfEnabledAndNotRunning in synchronized block. > 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)
