You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ud...@apache.org on 2016/06/15 16:17:42 UTC
[09/33] incubator-geode git commit: GEODE-1494: Changing stats list
to be a CopyOnWriteArrayList
GEODE-1494: Changing stats list to be a CopyOnWriteArrayList
The statsList used to track Statistics in InternalDistributedSystem was
managed under a lock. That was problematic, because code that iterates
on the list needs to hold the lock. If the code invoking callbacks for
GEODE-1494 was slow, that means the lock would block other stats
operations.
Changing this list to a copy on write array list to holding a lock for a
long period of time.
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/e9ffdce4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/e9ffdce4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/e9ffdce4
Branch: refs/heads/feature/GEODE-420
Commit: e9ffdce4fdf4f5c3b9f4ae45e19168e3de55ebe9
Parents: 5e0050d
Author: Dan Smith <up...@apache.org>
Authored: Tue Jun 7 11:44:24 2016 -0700
Committer: Dan Smith <up...@apache.org>
Committed: Mon Jun 13 10:38:36 2016 -0700
----------------------------------------------------------------------
.../internal/InternalDistributedSystem.java | 36 ++++-------
.../internal/AbstractStatisticsFactory.java | 65 ++++++++------------
.../gemfire/internal/GemFireStatSampler.java | 14 ++---
.../gemfire/internal/HostStatSampler.java | 10 ++-
.../cache/control/HeapMemoryMonitor.java | 20 +++---
5 files changed, 59 insertions(+), 86 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e9ffdce4/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalDistributedSystem.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalDistributedSystem.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalDistributedSystem.java
index 67f5478..a672127 100755
--- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalDistributedSystem.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalDistributedSystem.java
@@ -1668,7 +1668,7 @@ public class InternalDistributedSystem
return sb.toString().trim();
}
- private final ArrayList<Statistics> statsList = new ArrayList<Statistics>();
+ private final CopyOnWriteArrayList<Statistics> statsList = new CopyOnWriteArrayList<Statistics>();
private int statsListModCount = 0;
private long statsListUniqueId = 1;
private final Object statsListUniqueIdLock = new Object();
@@ -1698,11 +1698,9 @@ public class InternalDistributedSystem
@Override
public final Statistics findStatistics(long id) {
List<Statistics> statsList = this.statsList;
- synchronized (statsList) {
- for (Statistics s : statsList) {
- if (s.getUniqueId() == id) {
- return s;
- }
+ for (Statistics s : statsList) {
+ if (s.getUniqueId() == id) {
+ return s;
}
}
throw new RuntimeException(LocalizedStrings.PureStatSampler_COULD_NOT_FIND_STATISTICS_INSTANCE.toLocalizedString());
@@ -1711,11 +1709,9 @@ public class InternalDistributedSystem
@Override
public final boolean statisticsExists(long id) {
List<Statistics> statsList = this.statsList;
- synchronized (statsList) {
- for (Statistics s : statsList) {
- if (s.getUniqueId() == id) {
- return true;
- }
+ for (Statistics s : statsList) {
+ if (s.getUniqueId() == id) {
+ return true;
}
}
return false;
@@ -1724,9 +1720,7 @@ public class InternalDistributedSystem
@Override
public final Statistics[] getStatistics() {
List<Statistics> statsList = this.statsList;
- synchronized (statsList) {
- return (Statistics[])statsList.toArray(new Statistics[statsList.size()]);
- }
+ return (Statistics[])statsList.toArray(new Statistics[statsList.size()]);
}
// StatisticsFactory methods
@@ -1786,10 +1780,8 @@ public class InternalDistributedSystem
* This method was added to fix bug 40358
*/
public void visitStatistics(StatisticsVisitor visitor) {
- synchronized (this.statsList) {
- for (Statistics s: this.statsList) {
- visitor.visit(s);
- }
+ for (Statistics s: this.statsList) {
+ visitor.visit(s);
}
}
@@ -1844,11 +1836,9 @@ public class InternalDistributedSystem
return (Statistics[])hits.toArray(result);
}
public Statistics findStatisticsByUniqueId(final long uniqueId) {
- synchronized (this.statsList) {
- for (Statistics s: this.statsList) {
- if (uniqueId == s.getUniqueId()) {
- return s;
- }
+ for (Statistics s: this.statsList) {
+ if (uniqueId == s.getUniqueId()) {
+ return s;
}
}
return null;
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e9ffdce4/geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractStatisticsFactory.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractStatisticsFactory.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractStatisticsFactory.java
index 886258b..972e670 100755
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractStatisticsFactory.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractStatisticsFactory.java
@@ -28,6 +28,7 @@ import java.io.Reader;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
/**
* An abstract standalone implementation of {@link StatisticsFactory}.
@@ -41,7 +42,7 @@ public abstract class AbstractStatisticsFactory
private final long id;
private final String name;
- private final List<Statistics> statsList;
+ private final CopyOnWriteArrayList<Statistics> statsList;
private int statsListModCount = 0;
private long statsListUniqueId = 1;
private final Object statsListUniqueIdLock;
@@ -53,7 +54,7 @@ public abstract class AbstractStatisticsFactory
this.name = name;
this.startTime = startTime;
- this.statsList = new ArrayList<Statistics>();
+ this.statsList = new CopyOnWriteArrayList<Statistics>();
this.statsListUniqueIdLock = new Object();
this.tf = StatisticsTypeFactoryImpl.singleton();
}
@@ -112,11 +113,9 @@ public abstract class AbstractStatisticsFactory
@Override
public final boolean statisticsExists(long id) {
List<Statistics> statsList = this.statsList;
- synchronized (statsList) {
- for (Statistics s : statsList) {
- if (s.getUniqueId() == id) {
- return true;
- }
+ for (Statistics s : statsList) {
+ if (s.getUniqueId() == id) {
+ return true;
}
}
return false;
@@ -125,9 +124,7 @@ public abstract class AbstractStatisticsFactory
@Override
public final Statistics[] getStatistics() {
List<Statistics> statsList = this.statsList;
- synchronized (statsList) {
- return (Statistics[])statsList.toArray(new Statistics[statsList.size()]);
- }
+ return (Statistics[])statsList.toArray(new Statistics[statsList.size()]);
}
// StatisticsFactory methods
@@ -163,13 +160,11 @@ public abstract class AbstractStatisticsFactory
@Override
public final Statistics[] findStatisticsByType(StatisticsType type) {
List<Statistics> hits = new ArrayList<Statistics>();
- synchronized (statsList) {
- Iterator<Statistics> it = statsList.iterator();
- while (it.hasNext()) {
- Statistics s = (Statistics)it.next();
- if (type == s.getType()) {
- hits.add(s);
- }
+ Iterator<Statistics> it = statsList.iterator();
+ while (it.hasNext()) {
+ Statistics s = (Statistics)it.next();
+ if (type == s.getType()) {
+ hits.add(s);
}
}
Statistics[] result = new Statistics[hits.size()];
@@ -179,13 +174,11 @@ public abstract class AbstractStatisticsFactory
@Override
public final Statistics[] findStatisticsByTextId(String textId) {
List<Statistics> hits = new ArrayList<Statistics>();
- synchronized (statsList) {
- Iterator<Statistics> it = statsList.iterator();
- while (it.hasNext()) {
- Statistics s = (Statistics)it.next();
- if (s.getTextId().equals(textId)) {
- hits.add(s);
- }
+ Iterator<Statistics> it = statsList.iterator();
+ while (it.hasNext()) {
+ Statistics s = (Statistics)it.next();
+ if (s.getTextId().equals(textId)) {
+ hits.add(s);
}
}
Statistics[] result = new Statistics[hits.size()];
@@ -195,13 +188,11 @@ public abstract class AbstractStatisticsFactory
@Override
public final Statistics[] findStatisticsByNumericId(long numericId) {
List<Statistics> hits = new ArrayList<Statistics>();
- synchronized (statsList) {
- Iterator<Statistics> it = statsList.iterator();
- while (it.hasNext()) {
- Statistics s = (Statistics)it.next();
- if (numericId == s.getNumericId()) {
- hits.add(s);
- }
+ Iterator<Statistics> it = statsList.iterator();
+ while (it.hasNext()) {
+ Statistics s = (Statistics)it.next();
+ if (numericId == s.getNumericId()) {
+ hits.add(s);
}
}
Statistics[] result = new Statistics[hits.size()];
@@ -209,13 +200,11 @@ public abstract class AbstractStatisticsFactory
}
public final Statistics findStatisticsByUniqueId(long uniqueId) {
- synchronized (statsList) {
- Iterator<Statistics> it = statsList.iterator();
- while (it.hasNext()) {
- Statistics s = (Statistics)it.next();
- if (uniqueId == s.getUniqueId()) {
- return s;
- }
+ Iterator<Statistics> it = statsList.iterator();
+ while (it.hasNext()) {
+ Statistics s = (Statistics)it.next();
+ if (uniqueId == s.getUniqueId()) {
+ return s;
}
}
return null;
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e9ffdce4/geode-core/src/main/java/com/gemstone/gemfire/internal/GemFireStatSampler.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/GemFireStatSampler.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/GemFireStatSampler.java
index 5930506..1eb35d0 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/GemFireStatSampler.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/GemFireStatSampler.java
@@ -276,14 +276,12 @@ public final class GemFireStatSampler extends HostStatSampler {
}
if (stopRequested()) return;
HostStatHelper.readyRefreshOSStats();
- synchronized (l) {
- Iterator<Statistics> it = l.iterator();
- while (it.hasNext()) {
- if (stopRequested()) return;
- StatisticsImpl s = (StatisticsImpl)it.next();
- if (s.usesSystemCalls()) {
- HostStatHelper.refresh((LocalStatisticsImpl)s);
- }
+ Iterator<Statistics> it = l.iterator();
+ while (it.hasNext()) {
+ if (stopRequested()) return;
+ StatisticsImpl s = (StatisticsImpl)it.next();
+ if (s.usesSystemCalls()) {
+ HostStatHelper.refresh((LocalStatisticsImpl)s);
}
}
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e9ffdce4/geode-core/src/main/java/com/gemstone/gemfire/internal/HostStatSampler.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/HostStatSampler.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/HostStatSampler.java
index ebb4177..a5a07ca 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/HostStatSampler.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/HostStatSampler.java
@@ -510,12 +510,10 @@ public abstract class HostStatSampler
*/
private void sampleSpecialStats(boolean prepareOnly) {
List<Statistics> statsList = getStatisticsManager().getStatsList();
- synchronized (statsList) {
- for (Statistics s : statsList) {
- if (stopRequested()) return;
- if (s instanceof StatisticsImpl) {
- ((StatisticsImpl)s).prepareForSample();
- }
+ for (Statistics s : statsList) {
+ if (stopRequested()) return;
+ if (s instanceof StatisticsImpl) {
+ ((StatisticsImpl)s).prepareForSample();
}
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e9ffdce4/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/control/HeapMemoryMonitor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/control/HeapMemoryMonitor.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/control/HeapMemoryMonitor.java
index 90a26cb..bc83335 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/control/HeapMemoryMonitor.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/control/HeapMemoryMonitor.java
@@ -304,18 +304,16 @@ public void stopMonitoring() {
sampler.waitForInitialization();
String tenuredPoolName = getTenuredMemoryPoolMXBean().getName();
List list = this.cache.getDistributedSystem().getStatsList();
- synchronized (list) {
- for (Object o : list) {
- if (o instanceof StatisticsImpl) {
- StatisticsImpl si = (StatisticsImpl) o;
- if (si.getTextId().contains(tenuredPoolName) && si.getType().getName().contains("PoolStats")) {
- sampler.addLocalStatListener(this.statListener, si, "currentUsedMemory");
- if (this.cache.getLoggerI18n().fineEnabled()) {
- this.cache.getLoggerI18n().fine("Registered stat listener for " + si.getTextId());
- }
-
- return true;
+ for (Object o : list) {
+ if (o instanceof StatisticsImpl) {
+ StatisticsImpl si = (StatisticsImpl) o;
+ if (si.getTextId().contains(tenuredPoolName) && si.getType().getName().contains("PoolStats")) {
+ sampler.addLocalStatListener(this.statListener, si, "currentUsedMemory");
+ if (this.cache.getLoggerI18n().fineEnabled()) {
+ this.cache.getLoggerI18n().fine("Registered stat listener for " + si.getTextId());
}
+
+ return true;
}
}
}