From: Matthew Khouzam <matthew.khouzam@ericsson.com>
Date: Mon, 21 Sep 2015 20:06:28 +0000 (-0400)
Subject: segments: make TreeMapStore thread safe
X-Git-Url: http://git.efficios.com/?a=commitdiff_plain;h=71e78f69aa555b47dbffa42b55809b30dc27233d;p=deliverable%2Ftracecompass.git

segments: make TreeMapStore thread safe

This change will allow querying/iterating over the TreeMapStore is being built.
The data is not always accurate, but it is true to the time of the query.

Change-Id: I8721002fda68f7e88cb9ec742d76568a8c53e9ae
Signed-off-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Reviewed-on: https://git.eclipse.org/r/56381
Reviewed-by: Alexandre Montplaisir <alexmonthy@voxpopuli.im>
Tested-by: Alexandre Montplaisir <alexmonthy@voxpopuli.im>
Reviewed-by: Hudson CI
---

diff --git a/statesystem/org.eclipse.tracecompass.segmentstore.core/src/org/eclipse/tracecompass/segmentstore/core/treemap/TreeMapStore.java b/statesystem/org.eclipse.tracecompass.segmentstore.core/src/org/eclipse/tracecompass/segmentstore/core/treemap/TreeMapStore.java
index e6a0f93897..e0acbab718 100644
--- a/statesystem/org.eclipse.tracecompass.segmentstore.core/src/org/eclipse/tracecompass/segmentstore/core/treemap/TreeMapStore.java
+++ b/statesystem/org.eclipse.tracecompass.segmentstore.core/src/org/eclipse/tracecompass/segmentstore/core/treemap/TreeMapStore.java
@@ -16,6 +16,8 @@ import static org.eclipse.tracecompass.common.core.NonNullUtils.checkNotNull;
 
 import java.util.Iterator;
 import java.util.TreeMap;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.eclipse.tracecompass.segmentstore.core.ISegment;
 import org.eclipse.tracecompass.segmentstore.core.ISegmentStore;
@@ -47,10 +49,12 @@ import com.google.common.collect.TreeMultimap;
  */
 public class TreeMapStore<T extends ISegment> implements ISegmentStore<T> {
 
+    private final ReadWriteLock fLock = new ReentrantReadWriteLock(false);
+
     private final TreeMultimap<Long, T> fStartTimesIndex;
     private final TreeMultimap<Long, T> fEndTimesIndex;
 
-    private volatile long fSize;
+    private long fSize;
 
     /**
      * Constructor
@@ -79,22 +83,36 @@ public class TreeMapStore<T extends ISegment> implements ISegmentStore<T> {
         fSize = 0;
     }
 
+    /**
+     * Warning, this is not thread safe, and can cause concurrent modification
+     * exceptions
+     */
     @Override
     public Iterator<T> iterator() {
         return checkNotNull(fStartTimesIndex.values().iterator());
     }
 
     @Override
-    public synchronized void addElement(T val) {
-        if (fStartTimesIndex.put(Long.valueOf(val.getStart()), val)) {
-            fEndTimesIndex.put(Long.valueOf(val.getEnd()), val);
-            fSize++;
+    public void addElement(T val) {
+        fLock.writeLock().lock();
+        try {
+            if (fStartTimesIndex.put(Long.valueOf(val.getStart()), val)) {
+                fEndTimesIndex.put(Long.valueOf(val.getEnd()), val);
+                fSize++;
+            }
+        } finally {
+            fLock.writeLock().unlock();
         }
     }
 
     @Override
     public long getNbElements() {
-        return fSize;
+        fLock.readLock().lock();
+        try {
+            return fSize;
+        } finally {
+            fLock.readLock().unlock();
+        }
     }
 
     @Override
@@ -103,24 +121,37 @@ public class TreeMapStore<T extends ISegment> implements ISegmentStore<T> {
          * The intervals intersecting 't' are those whose 1) start time is
          * *lower* than 't' AND 2) end time is *higher* than 't'.
          */
-        Iterable<T> matchStarts = Iterables.concat(fStartTimesIndex.asMap().headMap(position, true).values());
-        Iterable<T> matchEnds = Iterables.concat(fEndTimesIndex.asMap().tailMap(position, true).values());
-
-        return checkNotNull(Sets.intersection(Sets.newHashSet(matchStarts), Sets.newHashSet(matchEnds)));
+        fLock.readLock().lock();
+        try {
+            Iterable<T> matchStarts = Iterables.concat(fStartTimesIndex.asMap().headMap(position, true).values());
+            Iterable<T> matchEnds = Iterables.concat(fEndTimesIndex.asMap().tailMap(position, true).values());
+            return checkNotNull(Sets.intersection(Sets.newHashSet(matchStarts), Sets.newHashSet(matchEnds)));
+        } finally {
+            fLock.readLock().unlock();
+        }
     }
 
     @Override
     public Iterable<T> getIntersectingElements(long start, long end) {
-        Iterable<T> matchStarts = Iterables.concat(fStartTimesIndex.asMap().headMap(end, true).values());
-        Iterable<T> matchEnds = Iterables.concat(fEndTimesIndex.asMap().tailMap(start, true).values());
-
-        return checkNotNull(Sets.intersection(Sets.newHashSet(matchStarts), Sets.newHashSet(matchEnds)));
+        fLock.readLock().lock();
+        try {
+            Iterable<T> matchStarts = Iterables.concat(fStartTimesIndex.asMap().headMap(end, true).values());
+            Iterable<T> matchEnds = Iterables.concat(fEndTimesIndex.asMap().tailMap(start, true).values());
+            return checkNotNull(Sets.intersection(Sets.newHashSet(matchStarts), Sets.newHashSet(matchEnds)));
+        } finally {
+            fLock.readLock().unlock();
+        }
     }
 
     @Override
-    public synchronized void dispose() {
-        fStartTimesIndex.clear();
-        fEndTimesIndex.clear();
-        fSize = 0;
+    public void dispose() {
+        fLock.writeLock().lock();
+        try {
+            fStartTimesIndex.clear();
+            fEndTimesIndex.clear();
+            fSize = 0;
+        } finally {
+            fLock.writeLock().unlock();
+        }
     }
 }