You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ad...@apache.org on 2016/12/02 01:29:56 UTC
incubator-geode git commit: GEODE-2141: Replace use of List with
ConcurrentHashSet for storing various stats and listener. Moved methods
stopNotifier_IfEnabledAndRunning and startNotifier_IfEnabledAndNotRunning in
synchronized block.
Repository: incubator-geode
Updated Branches:
refs/heads/develop c61028966 -> 50320dc4e
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.
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/50320dc4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/50320dc4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/50320dc4
Branch: refs/heads/develop
Commit: 50320dc4e828c900b6e1ee92c909fa019cae26d0
Parents: c610289
Author: adongre <av...@ampool.io>
Authored: Sun Nov 27 19:41:48 2016 +0530
Committer: adongre <av...@ampool.io>
Committed: Fri Dec 2 06:48:35 2016 +0530
----------------------------------------------------------------------
.../internal/statistics/StatMonitorHandler.java | 34 +++++--------
.../internal/statistics/StatisticsMonitor.java | 51 +++++++-------------
2 files changed, 29 insertions(+), 56 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/50320dc4/geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java
index 0c9583e..170ba49 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java
@@ -16,12 +16,11 @@ package org.apache.geode.internal.statistics;
import org.apache.geode.SystemFailure;
import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.concurrent.ConcurrentHashSet;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.logging.log4j.LogMarker;
import org.apache.logging.log4j.Logger;
-import java.util.ArrayList;
-import java.util.Collections;
import java.util.List;
import java.util.concurrent.SynchronousQueue;
@@ -38,7 +37,8 @@ public class StatMonitorHandler implements SampleHandler {
private final boolean enableMonitorThread;
/** The registered monitors */
- private volatile List<StatisticsMonitor> monitors = Collections.<StatisticsMonitor>emptyList();
+ private final ConcurrentHashSet<StatisticsMonitor> monitors =
+ new ConcurrentHashSet<StatisticsMonitor>();
/** Protected by synchronization on this handler instance */
private volatile StatMonitorNotifier notifier;
@@ -52,11 +52,8 @@ public class StatMonitorHandler implements SampleHandler {
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.contains(monitor)) {
+ added = this.monitors.add(monitor);
}
if (!this.monitors.isEmpty()) {
startNotifier_IfEnabledAndNotRunning();
@@ -69,11 +66,8 @@ public class StatMonitorHandler implements SampleHandler {
public boolean removeMonitor(StatisticsMonitor monitor) {
synchronized (this) {
boolean removed = false;
- List<StatisticsMonitor> oldMonitors = this.monitors;
- if (oldMonitors.contains(monitor)) {
- List<StatisticsMonitor> newMonitors = new ArrayList<StatisticsMonitor>(oldMonitors);
- removed = newMonitors.remove(monitor);
- this.monitors = Collections.unmodifiableList(newMonitors);
+ if (this.monitors.contains(monitor)) {
+ removed = this.monitors.remove(monitor);
}
if (this.monitors.isEmpty()) {
stopNotifier_IfEnabledAndRunning();
@@ -110,8 +104,7 @@ public class StatMonitorHandler implements SampleHandler {
}
private void monitor(final long sampleTimeMillis, final List<ResourceInstance> resourceInstance) {
- List<StatisticsMonitor> currentMonitors = StatMonitorHandler.this.monitors;
- for (StatisticsMonitor monitor : currentMonitors) {
+ for (StatisticsMonitor monitor : StatMonitorHandler.this.monitors) {
try {
monitor.monitor(sampleTimeMillis, resourceInstance);
} catch (VirtualMachineError e) {
@@ -138,15 +131,13 @@ public class StatMonitorHandler implements SampleHandler {
public void destroyedResourceInstance(ResourceInstance resourceInstance) {}
/** For testing only */
- List<StatisticsMonitor> getMonitorsSnapshot() {
- return Collections.unmodifiableList(this.monitors);
+ ConcurrentHashSet<StatisticsMonitor> getMonitorsSnapshot() {
+ return this.monitors;
}
/** For testing only */
StatMonitorNotifier getStatMonitorNotifier() {
- synchronized (this) {
- return this.notifier;
- }
+ return this.notifier;
}
private void startNotifier_IfEnabledAndNotRunning() {
@@ -228,8 +219,7 @@ public class StatMonitorHandler implements SampleHandler {
}
}
if (working && latestTask != null) {
- List<StatisticsMonitor> currentMonitors = StatMonitorHandler.this.monitors;
- for (StatisticsMonitor monitor : currentMonitors) {
+ for (StatisticsMonitor monitor : StatMonitorHandler.this.monitors) {
try {
monitor.monitor(latestTask.getSampleTimeMillis(),
latestTask.getResourceInstances());
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/50320dc4/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java
index 0e53557..16f71e2 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java
@@ -14,8 +14,8 @@
*/
package org.apache.geode.internal.statistics;
-import java.util.ArrayList;
-import java.util.Collections;
+import org.apache.geode.internal.concurrent.ConcurrentHashSet;
+
import java.util.List;
/**
@@ -29,9 +29,10 @@ public abstract class StatisticsMonitor {
private final Object mutex = new Object();
- private volatile List<StatisticsListener> listeners = Collections.<StatisticsListener>emptyList();
+ private final ConcurrentHashSet<StatisticsListener> listeners =
+ new ConcurrentHashSet<StatisticsListener>();
- private volatile List<StatisticId> statisticIds = Collections.<StatisticId>emptyList();
+ private final ConcurrentHashSet<StatisticId> statisticIds = new ConcurrentHashSet<StatisticId>();
public StatisticsMonitor() {}
@@ -39,13 +40,8 @@ public abstract class StatisticsMonitor {
if (statId == null) {
throw new NullPointerException("StatisticId is null");
}
- synchronized (this.mutex) {
- List<StatisticId> oldStatisticIds = this.statisticIds;
- if (!oldStatisticIds.contains(statId)) {
- List<StatisticId> newStatisticIds = new ArrayList<StatisticId>(oldStatisticIds);
- newStatisticIds.add(statId);
- this.statisticIds = Collections.unmodifiableList(newStatisticIds);
- }
+ if (!this.statisticIds.contains(statId)) {
+ this.statisticIds.add(statId);
}
return this;
}
@@ -54,13 +50,8 @@ public abstract class StatisticsMonitor {
if (statId == null) {
throw new NullPointerException("StatisticId is null");
}
- synchronized (this.mutex) {
- List<StatisticId> oldStatisticIds = this.statisticIds;
- if (oldStatisticIds.contains(statId)) {
- List<StatisticId> newStatisticIds = new ArrayList<StatisticId>(oldStatisticIds);
- newStatisticIds.remove(statId);
- this.statisticIds = Collections.unmodifiableList(newStatisticIds);
- }
+ if (this.statisticIds.contains(statId)) {
+ this.statisticIds.remove(statId);
}
return this;
}
@@ -70,11 +61,8 @@ public abstract class StatisticsMonitor {
throw new NullPointerException("StatisticsListener is null");
}
synchronized (this.mutex) {
- List<StatisticsListener> oldListeners = this.listeners;
- if (!oldListeners.contains(listener)) {
- List<StatisticsListener> newListeners = new ArrayList<StatisticsListener>(oldListeners);
- newListeners.add(listener);
- this.listeners = Collections.unmodifiableList(newListeners);
+ if (!this.listeners.contains(listener)) {
+ this.listeners.add(listener);
getStatMonitorHandler().addMonitor(this);
}
}
@@ -85,18 +73,15 @@ public abstract class StatisticsMonitor {
throw new NullPointerException("StatisticsListener is null");
}
synchronized (this.mutex) {
- List<StatisticsListener> oldListeners = this.listeners;
- if (oldListeners.contains(listener)) {
- List<StatisticsListener> newListeners = new ArrayList<StatisticsListener>(oldListeners);
- newListeners.remove(listener);
- if (newListeners.isEmpty()) {
+ if (this.listeners.contains(listener)) {
+ this.listeners.remove(listener);
+ if (this.listeners.isEmpty()) {
try {
getStatMonitorHandler().removeMonitor(this);
} catch (IllegalStateException ignore) {
// sample collector and handlers were closed (ok on removal)
}
}
- this.listeners = Collections.unmodifiableList(newListeners);
}
}
}
@@ -114,15 +99,13 @@ public abstract class StatisticsMonitor {
private final void monitorStatisticIds(long millisTimeStamp,
List<ResourceInstance> resourceInstances) {
- List<StatisticId> statisticIdsToMonitor = statisticIds;
- if (!statisticIdsToMonitor.isEmpty()) {
+ if (!this.statisticIds.isEmpty()) {
// TODO:
}
}
protected final void notifyListeners(StatisticsNotification notification) {
- List<StatisticsListener> listenersToNotify = this.listeners;
- for (StatisticsListener listener : listenersToNotify) {
+ for (StatisticsListener listener : this.listeners) {
listener.handleNotification(notification);
}
}
@@ -136,7 +119,7 @@ public abstract class StatisticsMonitor {
}
/** For testing only */
- List<StatisticsListener> getStatisticsListenersSnapshot() {
+ ConcurrentHashSet<StatisticsListener> getStatisticsListenersSnapshot() {
return this.listeners;
}