Skip to content

Commit 5f707e4

Browse files
abrahamwolkabrahamwolk
committed
CSSTUDIO-3420 Change type from TreeMap to ConcurrentSkipListMap, and remove synchronization on pvData.
1 parent d6d8ea4 commit 5f707e4

File tree

3 files changed

+66
-84
lines changed

3 files changed

+66
-84
lines changed

‎app/display/waterfallplot/src/main/java/org/phoebus/applications/waterfallplotwidget/WaterfallPlotController.java‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Optional;
2323
import java.util.SortedMap;
2424
import java.util.TreeMap;
25+
import java.util.concurrent.ConcurrentSkipListMap;
2526
import java.util.concurrent.atomic.AtomicBoolean;
2627
import java.util.concurrent.atomic.AtomicReference;
2728

@@ -320,21 +321,24 @@ public synchronized void redraw(WaterfallPlotRuntime.PVData pvData) {
320321
minFromPV = waveformPVData.minFromPV().get();
321322
maxFromPV = waveformPVData.maxFromPV().get();
322323

323-
TreeMap<Instant, LinkedList<Double>> instantToWaveform = waveformPVData.instantToValue().get();
324+
ConcurrentSkipListMap<Instant, LinkedList<Double>> instantToWaveform = waveformPVData.instantToValue();
325+
Instant startKey = instantToWaveform.ceilingKey(Instant.MIN);
324326
for (Instant t = t1.plus(stepsize); t.compareTo(t2) <= 0; t = t.plus(stepsize)) {
325327
timeValuesLinkedList.add(((double) t.toEpochMilli()) / 1000.0);
326328

327-
var instant = instantToWaveform.floorKey(t);
328-
if (instant != null) {
329+
if (startKey == null || t.isBefore(startKey)) {
330+
zValuesLinkedList.add(null); // null means absence of data for this point in time
331+
}
332+
else {
333+
var instant = instantToWaveform.floorKey(t);
334+
329335
LinkedList<Double> waveform = instantToWaveform.get(instant);
330336
waveformLength = Math.max(waveformLength, waveform.size());
331337
zValuesLinkedList.add(waveform);
332-
} else {
333-
zValuesLinkedList.add(null); // null means absence of data for this point in time
334338
}
335339
}
336340
} else if (pvData instanceof WaterfallPlotRuntime.ScalarPVsData scalarPVsData) {
337-
LinkedList<Pair<String, AtomicReference<TreeMap<Instant, Double>>>> pvNameToInstantToValue = scalarPVsData.pvNameToInstantToValue();
341+
LinkedList<Pair<String, ConcurrentSkipListMap<Instant, Double>>> pvNameToInstantToValue = scalarPVsData.pvNameToInstantToValue();
338342
pvNameToInstantToValue.forEach(pvNameAndInstantToValueAtomicReference -> garbageCollectInstantToValue(pvNameAndInstantToValueAtomicReference.getValue(), t1));
339343

340344
minFromPV = scalarPVsData.minFromPV().get();
@@ -349,7 +353,7 @@ public synchronized void redraw(WaterfallPlotRuntime.PVData pvData) {
349353
for (var pvNameAndInstantToValue : pvNameToInstantToValue) {
350354
String pvName = pvNameAndInstantToValue.getKey();
351355

352-
TreeMap<Instant, Double> instantToValue = pvNameAndInstantToValue.getValue().get();
356+
ConcurrentSkipListMap<Instant, Double> instantToValue = pvNameAndInstantToValue.getValue();
353357

354358
var instant = instantToValue.floorKey(t);
355359
if (instant == null) {
@@ -479,14 +483,13 @@ else if (zAxisMinMax.equals(WaterfallPlotWidget.ZAxisMinMax.FromPVLimits)) {
479483
}
480484
}
481485

482-
private synchronized static <T> void garbageCollectInstantToValue(AtomicReference<TreeMap<Instant, T>> instantToValueAtomicReference, Instant t1) {
486+
private synchronized static <T> void garbageCollectInstantToValue(ConcurrentSkipListMap<Instant, T> instantToWaveform, Instant t1) {
483487
// Garbage collect old values that are no longer needed:
484-
TreeMap<Instant, T> instantToWaveform = instantToValueAtomicReference.get();
485488
Instant instantOfOldestRelevantKey = instantToWaveform.floorKey(t1);
486489
if (instantOfOldestRelevantKey != null) {
487-
SortedMap<Instant, T> instantToWaveformGarbageCollectedSortedMap = instantToWaveform.subMap(instantOfOldestRelevantKey, Instant.MAX);
488-
TreeMap<Instant, T> instantToWaveformGarbageCollected = new TreeMap<>(instantToWaveformGarbageCollectedSortedMap);
489-
instantToValueAtomicReference.set(instantToWaveformGarbageCollected);
490+
for (var key : instantToWaveform.subMap(Instant.MIN, instantOfOldestRelevantKey).keySet()) {
491+
instantToWaveform.remove(key);
492+
}
490493
}
491494
}
492495

‎app/display/waterfallplot/src/main/java/org/phoebus/applications/waterfallplotwidget/WaterfallPlotRuntime.java‎

Lines changed: 47 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@
2424
import java.time.Instant;
2525
import java.util.LinkedList;
2626
import java.util.List;
27-
import java.util.TreeMap;
28-
import java.util.concurrent.atomic.AtomicReference;
27+
import java.util.concurrent.ConcurrentSkipListMap;
2928
import java.util.concurrent.locks.Lock;
3029
import java.util.function.Consumer;
3130
import java.util.logging.Level;
@@ -57,7 +56,7 @@ public void initialize(WaterfallPlotWidget waterfallPlotWidget) {
5756
if (isWaveform) {
5857
pvData = new WaveformPVData(new AtomicDouble(Double.NaN),
5958
new AtomicDouble(Double.NaN),
60-
new AtomicReference<>(new TreeMap<>()));
59+
new ConcurrentSkipListMap<>());
6160
}
6261
else {
6362
pvData = new ScalarPVsData(new AtomicDouble(Double.NaN),
@@ -69,16 +68,13 @@ public void initialize(WaterfallPlotWidget waterfallPlotWidget) {
6968
}
7069

7170
public sealed interface PVData permits WaveformPVData, ScalarPVsData {}
72-
// In order to be able to garbage collect data points that are no longer needed,
73-
// the TreeMap<> instantToValue is contained in an AtomicReference<>, so that we
74-
// can set the map to a new map without values that are no longer needed. This is
75-
// the case for both 'WaveformPVData' and 'ScalarPVsData'.
71+
// The type ConcurrentSkipListMap is used for the data points to allow for concurrent insertions and deletions:
7672
public record WaveformPVData (AtomicDouble minFromPV,
7773
AtomicDouble maxFromPV,
78-
AtomicReference<TreeMap<Instant, LinkedList<Double>>> instantToValue) implements PVData {}
74+
ConcurrentSkipListMap<Instant, LinkedList<Double>> instantToValue) implements PVData {}
7975
public record ScalarPVsData (AtomicDouble minFromPV,
8076
AtomicDouble maxFromPV,
81-
LinkedList<Pair<String, AtomicReference<TreeMap<Instant, Double>>>> pvNameToInstantToValue) implements PVData {}
77+
LinkedList<Pair<String, ConcurrentSkipListMap<Instant, Double>>> pvNameToInstantToValue) implements PVData {}
8278

8379
@Override
8480
public void start() {
@@ -90,32 +86,28 @@ public void start() {
9086
try {
9187
RuntimePV runtimePV = PVFactory.getPV(pvName);
9288
super.addPV(runtimePV, false);
93-
AtomicReference<TreeMap<Instant, Double>> instantToValueAtomicReference = new AtomicReference<>(new TreeMap<>());
94-
scalarPVsData.pvNameToInstantToValue.add(new Pair(pvName, instantToValueAtomicReference));
89+
ConcurrentSkipListMap<Instant, Double> instantToValue = new ConcurrentSkipListMap<>();
90+
scalarPVsData.pvNameToInstantToValue.add(new Pair(pvName, instantToValue));
9591
runtimePV.addListener((pv, vType) -> {
9692
if (vType instanceof VNumber vnumber) {
97-
synchronized (pvData) {
98-
instantToValueAtomicReference.get().put(vnumber.getTime().getTimestamp(), vnumber.getValue().doubleValue());
99-
{
100-
Range displayRange = vnumber.getDisplay().getDisplayRange();
101-
double minFromPV = displayRange.getMinimum();
102-
scalarPVsData.minFromPV.set(minFromPV);
103-
double maxFromPV = displayRange.getMaximum();
104-
scalarPVsData.maxFromPV.set(maxFromPV);
105-
}
93+
instantToValue.put(vnumber.getTime().getTimestamp(), vnumber.getValue().doubleValue());
94+
{
95+
Range displayRange = vnumber.getDisplay().getDisplayRange();
96+
double minFromPV = displayRange.getMinimum();
97+
scalarPVsData.minFromPV.set(minFromPV);
98+
double maxFromPV = displayRange.getMaximum();
99+
scalarPVsData.maxFromPV.set(maxFromPV);
106100
}
107101
}
108102
else if (vType instanceof VEnum vEnum) {
109-
synchronized (pvData) {
110-
instantToValueAtomicReference.get().put(vEnum.getTime().getTimestamp(), (double) vEnum.getIndex());
111-
112-
{
113-
int enumSize = vEnum.getDisplay().getChoices().size();
114-
double minFromPV = 0;
115-
scalarPVsData.minFromPV.set(minFromPV);
116-
double maxFromPV = enumSize - 1;
117-
scalarPVsData.maxFromPV.set(maxFromPV);
118-
}
103+
instantToValue.put(vEnum.getTime().getTimestamp(), (double) vEnum.getIndex());
104+
105+
{
106+
int enumSize = vEnum.getDisplay().getChoices().size();
107+
double minFromPV = 0;
108+
scalarPVsData.minFromPV.set(minFromPV);
109+
double maxFromPV = enumSize - 1;
110+
scalarPVsData.maxFromPV.set(maxFromPV);
119111
}
120112
}
121113
else {
@@ -128,18 +120,13 @@ else if (vType instanceof VEnum vEnum) {
128120
Instant.now().minusSeconds(timeSpanInSeconds),
129121
Instant.now(),
130122
values -> {
131-
synchronized (pvData) {
132-
for (var vtype : values) {
133-
if (vtype instanceof VNumber vnumber) {
134-
instantToValueAtomicReference.get().put(vnumber.getTime().getTimestamp(), vnumber.getValue().doubleValue());
135-
}
136-
else if (vtype instanceof VStatistics vstatistics) {
137-
instantToValueAtomicReference.get().put(vstatistics.getTime().getTimestamp(), vstatistics.getAverage());
138-
}
139-
else if (vtype instanceof VEnum vEnum) {
140-
instantToValueAtomicReference.get().put(vEnum.getTime().getTimestamp(), (double) vEnum.getIndex());
141-
}
142-
123+
for (var vtype : values) {
124+
if (vtype instanceof VNumber vnumber) {
125+
instantToValue.put(vnumber.getTime().getTimestamp(), vnumber.getValue().doubleValue());
126+
} else if (vtype instanceof VStatistics vstatistics) {
127+
instantToValue.put(vstatistics.getTime().getTimestamp(), vstatistics.getAverage());
128+
} else if (vtype instanceof VEnum vEnum) {
129+
instantToValue.put(vEnum.getTime().getTimestamp(), (double) vEnum.getIndex());
143130
}
144131
}
145132
});
@@ -162,20 +149,16 @@ else if (pvData instanceof WaveformPVData waveformPVData && pvNames.size() == 1)
162149
var value = vNumberArray.getData().getDouble(m);
163150
waveform.add(value);
164151
}
165-
166-
synchronized (pvData) {
167-
waveformPVData.instantToValue.get().put(vNumberArray.getTime().getTimestamp(), waveform);
168-
169-
{
170-
Range displayRange = vNumberArray.getDisplay().getDisplayRange();
171-
double minFromPV = displayRange.getMinimum();
172-
waveformPVData.minFromPV.set(minFromPV);
173-
double maxFromPV = displayRange.getMaximum();
174-
waveformPVData.maxFromPV.set(maxFromPV);
175-
}
152+
waveformPVData.instantToValue.put(vNumberArray.getTime().getTimestamp(), waveform);
153+
154+
{
155+
Range displayRange = vNumberArray.getDisplay().getDisplayRange();
156+
double minFromPV = displayRange.getMinimum();
157+
waveformPVData.minFromPV.set(minFromPV);
158+
double maxFromPV = displayRange.getMaximum();
159+
waveformPVData.maxFromPV.set(maxFromPV);
176160
}
177-
}
178-
else if (vType instanceof VEnumArray vEnumArray) {
161+
} else if (vType instanceof VEnumArray vEnumArray) {
179162

180163
LinkedList<Double> waveform = new LinkedList<>();
181164
ListNumber listNumber = vEnumArray.getIndexes();
@@ -184,16 +167,14 @@ else if (vType instanceof VEnumArray vEnumArray) {
184167
waveform.add(value);
185168
}
186169

187-
synchronized (pvData) {
188-
waveformPVData.instantToValue.get().put(vEnumArray.getTime().getTimestamp(), waveform);
170+
waveformPVData.instantToValue.put(vEnumArray.getTime().getTimestamp(), waveform);
189171

190-
{
191-
int enumSize = vEnumArray.getDisplay().getChoices().size();
192-
double minFromPV = 0;
193-
waveformPVData.minFromPV.set(minFromPV);
194-
double maxFromPV = enumSize - 1;
195-
waveformPVData.maxFromPV.set(maxFromPV);
196-
}
172+
{
173+
int enumSize = vEnumArray.getDisplay().getChoices().size();
174+
double minFromPV = 0;
175+
waveformPVData.minFromPV.set(minFromPV);
176+
double maxFromPV = enumSize - 1;
177+
waveformPVData.maxFromPV.set(maxFromPV);
197178
}
198179
}
199180
});
@@ -213,7 +194,7 @@ else if (vType instanceof VEnumArray vEnumArray) {
213194
waveform.add(value);
214195
}
215196

216-
waveformPVData.instantToValue.get().put(vNumberArray.getTime().getTimestamp(), waveform);
197+
waveformPVData.instantToValue.put(vNumberArray.getTime().getTimestamp(), waveform);
217198
}
218199
else if (vtype instanceof VEnumArray vEnumArray) {
219200

@@ -224,7 +205,7 @@ else if (vtype instanceof VEnumArray vEnumArray) {
224205
waveform.add(value);
225206
}
226207

227-
waveformPVData.instantToValue.get().put(vEnumArray.getTime().getTimestamp(), waveform);
208+
waveformPVData.instantToValue.put(vEnumArray.getTime().getTimestamp(), waveform);
228209
}
229210
}
230211
}

‎app/display/waterfallplot/src/main/java/org/phoebus/applications/waterfallplotwidget/WaterfallPlotWidgetRepresentation.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,10 @@ public WaterfallPlotWidgetRepresentation() {
110110
}
111111

112112
WaterfallPlotRuntime.PVData pvData = waterfallPlotRuntime.getPVData();
113-
synchronized (pvData) {
114-
try {
115-
waterfallPlotController.redraw(pvData);
116-
} catch (Exception e) {
117-
// Catch exceptions in order to retry redrawing again if an exception occurs.
118-
}
113+
try {
114+
waterfallPlotController.redraw(pvData);
115+
} catch (Exception e) {
116+
// Catch exceptions in order to retry redrawing again if an exception occurs.
119117
}
120118
};
121119

0 commit comments

Comments
 (0)