From 39c6fb4506a5fed5c067fdf3467198769b2e2a03 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 12:12:59 +0000 Subject: [PATCH 1/7] DEV-896 Add monotonic-X bounded trace to kill per-sample O(n) rescans Live time-series plots stutter because Trace2DLtd rescans the whole ring buffer (minXSearch) on every eviction: with strictly increasing X the evicted oldest point always holds minX, so this runs every sample, and it happens under synchronized(chart) - the monitor Chart2D.paintComponent() needs - starving the Swing EDT. Trace2DLtdMonotonicX overrides minXSearch/maxXSearch to O(1) (oldest/youngest ring element) whenever the buffer is known sorted ascending by X, tracking a non-decreasing-X run length so it falls back to the exact superclass scan on rewind/replay. Y is left to the superclass (already amortized O(1)). Swap the three live-plot construction sites in BasicPlotManagerPC; existing (Trace2DLtd) casts still work as it extends Trace2DLtd. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JjgNkS6wtPkQUdK3KdHVdN --- .../guiUtilities/plot/BasicPlotManagerPC.java | 6 +- .../plot/Trace2DLtdMonotonicX.java | 142 ++++++++++++++++++ 2 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/Trace2DLtdMonotonicX.java diff --git a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java index 095947bfc..e083ca5af 100644 --- a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java +++ b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java @@ -347,7 +347,7 @@ private ITrace2D addBarTrace(Chart2D chart, int plotMaxSize) { } private ITrace2D createNormalTrace(int plotMaxSize) { - Trace2DLtd trace = new Trace2DLtd(plotMaxSize); + Trace2DLtd trace = new Trace2DLtdMonotonicX(plotMaxSize); //DEV-896: monotonic-X trace avoids O(n) minX rescans per sample BasicStroke stroke = ((BasicStroke)trace.getStroke()); BasicStroke newStroke = new BasicStroke(DEFAULT_LINE_THICKNESS,stroke.getEndCap(),stroke.getLineJoin(),stroke.getMiterLimit(),stroke.getDashArray(),stroke.getDashPhase()); trace.setStroke(newStroke); @@ -355,7 +355,7 @@ private ITrace2D createNormalTrace(int plotMaxSize) { } private ITrace2D createBarTrace(Chart2D chart, int plotMaxSize) { - ITrace2D trace = new Trace2DLtd(plotMaxSize); + ITrace2D trace = new Trace2DLtdMonotonicX(plotMaxSize); //DEV-896: monotonic-X trace avoids O(n) minX rescans per sample trace.setTracePainter(new TracePainterVerticalBar(chart)); return trace; } @@ -369,7 +369,7 @@ private ITrace2D createBarTrace(Chart2D chart, int plotMaxSize) { */ private ITrace2D addSignalToExistingChartInternal(String[] signal, int plotMaxSize, Color color) throws Exception{ if (!checkIfPropertyExist(signal)){ - ITrace2D trace = new Trace2DLtd(plotMaxSize); + ITrace2D trace = new Trace2DLtdMonotonicX(plotMaxSize); //DEV-896: monotonic-X trace avoids O(n) minX rescans per sample mChart.addTrace(trace); mListofTraces.add(trace); diff --git a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/Trace2DLtdMonotonicX.java b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/Trace2DLtdMonotonicX.java new file mode 100644 index 000000000..754446b4c --- /dev/null +++ b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/Trace2DLtdMonotonicX.java @@ -0,0 +1,142 @@ +package com.shimmerresearch.guiUtilities.plot; + +import info.monitorenter.gui.chart.ITracePoint2D; +import info.monitorenter.gui.chart.traces.Trace2DLtd; + +/** + * DEV-896: A bounded (ring-buffer backed) trace optimised for the live time-series + * streaming case where the X value is monotonically (non-decreasing) increasing. + * + *

Problem being solved: {@code Trace2DLtd.addPointInternal()} maintains the trace + * min/max by, on every eviction of the oldest ring-buffer element, checking whether the + * evicted point held an extreme and if so running a full linear rescan + * ({@code ATrace2D.minXSearch()} / {@code maxXSearch()} / {@code minYSearch()} / + * {@code maxYSearch()}) over the whole buffer. For a time-series plot X is strictly + * increasing, so the evicted (oldest) point ALWAYS holds the minimum X, which triggers an + * O(buffer) {@code minXSearch()} on essentially every sample in steady state. Because + * {@code ATrace2D.addPoint()} does that work while holding {@code synchronized(chart)} + * (the same monitor {@code Chart2D.paintComponent()} uses), the data thread starves the + * Swing EDT and the plot stutters.

+ * + *

Fix: when the ring buffer is known to be sorted ascending by X, the minimum X is + * simply the oldest buffer element and the maximum X the youngest, both O(1). We override + * {@code minXSearch()} / {@code maxXSearch()} to use those accessors on the fast path and + * fall back to the (correct) superclass scan otherwise.

+ * + *

Correctness / robustness:

+ * + * + *

Externally this class behaves identically to {@code Trace2DLtd} (same bounds, same + * property-change events, same {@code setMaxSize} semantics); it only removes the redundant + * O(n) X rescans. It extends {@code Trace2DLtd} so existing + * {@code ((Trace2DLtd)trace).setMaxSize(...)} / {@code .iterator()} casts keep working.

+ */ +public class Trace2DLtdMonotonicX extends Trace2DLtd { + + /** X value of the most recently inserted point, used to detect non-decreasing X. */ + private double mLastX = Double.NaN; + + /** + * Number of consecutive insertions (ending at the most recent one) whose X was + * non-decreasing. When this is {@code >= m_buffer.size()} the entire current buffer + * content was produced by a non-decreasing run and is therefore sorted ascending by X. + */ + private long mAscendingRunLength = 0L; + + public Trace2DLtdMonotonicX() { + super(); + } + + public Trace2DLtdMonotonicX(int maxSize) { + super(maxSize); + } + + public Trace2DLtdMonotonicX(int maxSize, String name) { + super(maxSize, name); + } + + public Trace2DLtdMonotonicX(String name) { + super(name); + } + + /** + * @return {@code true} when the backing ring buffer is currently guaranteed to be sorted + * ascending by X, i.e. the most recent {@code size()} insertions were all + * non-decreasing in X. Reads the live buffer size so it stays correct across + * {@code setMaxSize(int)}. + */ + private boolean isBufferSortedAscendingByX() { + if (m_buffer == null || m_buffer.isEmpty()) { + return false; + } + return mAscendingRunLength >= m_buffer.size(); + } + + @Override + protected boolean addPointInternal(ITracePoint2D p) { + double x = p.getX(); + if (Double.isNaN(mLastX) || x >= mLastX) { + // Non-decreasing X: extend the ascending run (cap to avoid overflow; any value + // above the buffer size already means "fully sorted"). + if (mAscendingRunLength < Long.MAX_VALUE) { + mAscendingRunLength++; + } + } else { + // X went backwards: the buffer is no longer sorted. This incoming point starts a + // new ascending run of length 1. The fast path resumes once the run refills the + // buffer; until then the superclass scans keep the range exact. + mAscendingRunLength = 1L; + } + mLastX = x; + // Delegates to Trace2DLtd, which on eviction virtually dispatches to our overridden + // minXSearch()/maxXSearch() below (and to the unchanged Y searches). + return super.addPointInternal(p); + } + + @Override + protected void minXSearch() { + if (isBufferSortedAscendingByX()) { + try { + // Oldest element holds the smallest X when the buffer is sorted ascending. + m_minX = m_buffer.getOldest().getX(); + return; + } catch (RuntimeException e) { + // Buffer emptied concurrently / unexpected state: fall back to the safe scan. + } + } + super.minXSearch(); + } + + @Override + protected void maxXSearch() { + if (isBufferSortedAscendingByX()) { + try { + // Youngest element holds the largest X when the buffer is sorted ascending. + m_maxX = m_buffer.getYoungest().getX(); + return; + } catch (RuntimeException e) { + // Fall back to the safe scan. + } + } + super.maxXSearch(); + } +} From 5af0117a43158d4603c2d4804d9fe3e6c9fc5b10 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 12:13:59 +0000 Subject: [PATCH 2/7] DEV-896 Batch addPoint calls under one chart-monitor acquisition Each ATrace2D.addPoint() takes synchronized(chart) - the same monitor Chart2D.paintComponent() holds - so plotting a multi-channel sample or an FFT frame grabbed the monitor once per point, repeatedly locking out the EDT. filterDataAndPlot now wraps its per-sample multi-trace loop in a single synchronized(mChart) (monitors are reentrant, so addPoint's inner lock is free); the hold is bounded by the trace count. Add addPointsToTrace(...) that adds many points to one trace under one lock in bounded 256-point chunks, and use it for the FFT bin loop. Event-marker, downsampling and per-sample filtering semantics are unchanged; BasicProcessWithCallBack is untouched. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JjgNkS6wtPkQUdK3KdHVdN --- .../guiUtilities/plot/BasicPlotManagerPC.java | 72 +++++++++++++++++-- 1 file changed, 65 insertions(+), 7 deletions(-) diff --git a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java index e083ca5af..691898cf6 100644 --- a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java +++ b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java @@ -1645,7 +1645,56 @@ public void addPointToTrace(ITrace2D trace, double xData, double yData){ } trace.addPoint(xData, yData); } - + + /** DEV-896: Max points added per single chart-monitor acquisition in {@link #addPointsToTrace}. + * Bounds how long the batch can hold the chart lock so a large burst can't monopolise the EDT. */ + private static final int POINT_BATCH_MAX = 256; + + /** + * DEV-896: Batch variant of {@link #addPointToTrace(ITrace2D, double, double)}. Adds many + * points to a single trace while acquiring the chart monitor once per (bounded) chunk rather + * than once per point. {@code ATrace2D.addPoint()} synchronizes on the chart + * ({@code trace.getRenderer()}) - the same monitor {@code Chart2D.paintComponent()} holds - so + * adding N points individually took the monitor N times and starved the Swing EDT. Java monitors + * are reentrant, so {@code addPoint()}'s internal {@code synchronized(chart)} is free while we + * hold the outer lock. Behaviour per point is identical to {@link #addPointToTrace}. + */ + public void addPointsToTrace(ITrace2D trace, double[] xData, double[] yData){ + if(xData == null || yData == null){ + return; + } + addPointsToTrace(trace, xData, yData, 0, Math.min(xData.length, yData.length)); + } + + /** + * DEV-896: See {@link #addPointsToTrace(ITrace2D, double[], double[])}. Adds points + * {@code [fromIndex, toIndex)} from the given arrays. + */ + public void addPointsToTrace(ITrace2D trace, double[] xData, double[] yData, int fromIndex, int toIndex){ + if(trace == null || xData == null || yData == null){ + return; + } + int end = Math.min(toIndex, Math.min(xData.length, yData.length)); + int i = Math.max(0, fromIndex); + //trace.getRenderer() returns the Chart2D the trace was added to (null until then); it is the + //exact monitor ATrace2D.addPoint() locks, so holding it makes the per-point locks reentrant. + Chart2D chart = trace.getRenderer(); + while(i < end){ + int chunkEnd = Math.min(i + POINT_BATCH_MAX, end); + if(chart != null){ + synchronized(chart){ + for(; i < chunkEnd; i++){ + addPointToTrace(trace, xData[i], yData[i]); + } + } + } else { + for(; i < chunkEnd; i++){ + addPointToTrace(trace, xData[i], yData[i]); + } + } + } + } + public CircularFifoBuffer getCirculurBufferedTraceData(String traceName){ CircularFifoBuffer circularFifoBuffer = mMapOfCirculurBufferedTraceDataPoints.get(traceName); if(circularFifoBuffer != null){ @@ -1869,10 +1918,9 @@ public void run() { if(results.length==2 && results[0].length>startBin){ trace.removeAllPoints(); - - for(int x=startBin;x entries = mListofPropertiestoPlot.iterator(); int indexOfTrace = 0; - boolean isDummyPointAddedToFillTrace = false; - + boolean isDummyPointAddedToFillTrace = false; + + //DEV-896: Acquire the chart monitor once for this whole multi-trace update instead + //of once per trace inside ATrace2D.addPoint(). addPoint() synchronizes on the chart + //(trace.getRenderer()) - the same monitor Chart2D.paintComponent() holds - so grabbing + //it once per sample was starving the Swing EDT. Java monitors are reentrant, so the + //per-point synchronized(chart) inside addPoint() is free while we hold this outer lock. + //The batch is bounded by the number of plotted traces, so the lock hold stays short. + //Falls back to the already-held mListofPropertiestoPlot monitor if no chart is set yet. + Object chartMonitor = (mChart != null) ? (Object)mChart : (Object)mListofPropertiestoPlot; + synchronized(chartMonitor){ while (entries.hasNext()) { String[] props = entries.next(); @@ -2067,6 +2124,7 @@ else if(isXAxisFrequency){ } indexOfTrace++; } + } //DEV-896: release the chart monitor once the whole multi-trace update is done if(isDummyPointAddedToFillTrace) { isFirstPointOnFillTrace = false; } From ede3b34a41c4eb500c4c49b3742c4531231e439c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 12:31:12 +0000 Subject: [PATCH 3/7] DEV-896 review fixes: trace-list snapshot to prevent lock inversion, harden monotonic trace - filterDataAndPlot accessed mListofTraces (size/get) while holding the new chart monitor (chart -> list order), while clearAllDataBuffer and the trace-resize path hold the mListofTraces monitor while calling chart-locking trace mutators (list -> chart). Snapshot the trace list before entering the chart monitor so the cycle cannot form. - Trace2DLtdMonotonicX: NaN X (jchart2d discontinuation marker) now contributes nothing to the ascending run instead of being absorbed by the first-point guard; in-place point mutation (STATE_CHANGED) resets the run so the class stays a safe drop-in for Trace2DLtd; tracking fields made volatile so the unlocked conservative resets cannot tear and spuriously enable the fast path; corrected the setMaxSize-grow Javadoc reasoning. Correctness test: 11596 checks vs stock Trace2DLtd, 0 mismatches. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JjgNkS6wtPkQUdK3KdHVdN --- .../guiUtilities/plot/BasicPlotManagerPC.java | 10 ++++- .../plot/Trace2DLtdMonotonicX.java | 42 +++++++++++++++---- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java index 691898cf6..bb689b93d 100644 --- a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java +++ b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java @@ -2008,6 +2008,12 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { //per-point synchronized(chart) inside addPoint() is free while we hold this outer lock. //The batch is bounded by the number of plotted traces, so the lock hold stays short. //Falls back to the already-held mListofPropertiestoPlot monitor if no chart is set yet. + //IMPORTANT (lock ordering): snapshot mListofTraces BEFORE taking the chart monitor. + //Other threads (e.g. clearAllDataBuffer, trace resizing) hold the mListofTraces monitor + //while calling chart-locking trace mutators (removeAllPoints/setMaxSize), i.e. + //mListofTraces -> chart. Touching mListofTraces while holding the chart monitor here + //would be the reverse order and a real deadlock cycle. + ITrace2D[] tracesSnapshot = mListofTraces.toArray(new ITrace2D[0]); Object chartMonitor = (mChart != null) ? (Object)mChart : (Object)mListofPropertiestoPlot; synchronized(chartMonitor){ while (entries.hasNext()) { @@ -2068,10 +2074,10 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { continue; } - if (indexOfTrace>mListofTraces.size()){ + if (indexOfTrace>tracesSnapshot.length){ throw new Exception("Trace does not exist: (" + traceName + ")"); } - ITrace2D currentTrace = mListofTraces.get(indexOfTrace); + ITrace2D currentTrace = tracesSnapshot[indexOfTrace]; //utilShimmer.consolePrintErrLn(currentTrace.getMaxY()); mCurrentXValue = xData; diff --git a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/Trace2DLtdMonotonicX.java b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/Trace2DLtdMonotonicX.java index 754446b4c..fc585c186 100644 --- a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/Trace2DLtdMonotonicX.java +++ b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/Trace2DLtdMonotonicX.java @@ -40,9 +40,10 @@ * optimising it is unnecessary and would risk the displayed Y auto-scale. *
  • {@code setMaxSize(int)} is {@code final} in {@code Trace2DLtd} so it cannot be * overridden, but no reset hook is needed: {@link #isBufferSortedAscendingByX()} reads - * the live {@code m_buffer.size()} each call, so growing the buffer transparently - * disables the fast path until it refills and shrinking (which only discards the - * oldest, smallest-X elements) keeps the buffer sorted.
    • + * the live {@code m_buffer.size()} each call. Growing leaves the element count and + * ordering unchanged (a sorted buffer stays sorted, so the fast path validly stays + * available); shrinking only discards the oldest, smallest-X elements, which also + * keeps the buffer sorted. * * *

    Externally this class behaves identically to {@code Trace2DLtd} (same bounds, same @@ -52,15 +53,21 @@ */ public class Trace2DLtdMonotonicX extends Trace2DLtd { - /** X value of the most recently inserted point, used to detect non-decreasing X. */ - private double mLastX = Double.NaN; + /** + * X value of the most recently inserted point, used to detect non-decreasing X. + * Volatile: normal updates happen under the chart+trace locks (inside addPoint), but the + * conservative resets in {@link #firePointChanged} may run outside them; volatile prevents + * a torn 64-bit write from ever spuriously enabling the fast path. All unlocked writes are + * resets, which can only (safely) disable it. + */ + private volatile double mLastX = Double.NaN; /** * Number of consecutive insertions (ending at the most recent one) whose X was * non-decreasing. When this is {@code >= m_buffer.size()} the entire current buffer * content was produced by a non-decreasing run and is therefore sorted ascending by X. */ - private long mAscendingRunLength = 0L; + private volatile long mAscendingRunLength = 0L; public Trace2DLtdMonotonicX() { super(); @@ -94,7 +101,12 @@ private boolean isBufferSortedAscendingByX() { @Override protected boolean addPointInternal(ITracePoint2D p) { double x = p.getX(); - if (Double.isNaN(mLastX) || x >= mLastX) { + if (Double.isNaN(x)) { + // NaN X (jchart2d's discontinuation marker) breaks any ordering guarantee for as + // long as it stays in the buffer: contribute nothing to the ascending run, so the + // fast path can only resume once a full buffer of post-NaN points has evicted it. + mAscendingRunLength = 0L; + } else if (Double.isNaN(mLastX) || x >= mLastX) { // Non-decreasing X: extend the ascending run (cap to avoid overflow; any value // above the buffer size already means "fully sorted"). if (mAscendingRunLength < Long.MAX_VALUE) { @@ -112,6 +124,22 @@ protected boolean addPointInternal(ITracePoint2D p) { return super.addPointInternal(p); } + /** + * In-place mutation of an existing point ({@code ITracePoint2D.setLocation}) fires a + * {@code STATE_CHANGED} notification and can reorder the buffer arbitrarily, which the + * insertion-time run tracking cannot see. Reset the run so the fast path stays off until + * a full buffer of fresh monotonic insertions restores the guarantee. (Not used by the + * Shimmer streaming paths, but keeps this class a safe drop-in for Trace2DLtd.) + */ + @Override + public void firePointChanged(final ITracePoint2D changed, final int state) { + if (state == ITracePoint2D.STATE_CHANGED) { + mAscendingRunLength = 0L; + mLastX = Double.NaN; + } + super.firePointChanged(changed, state); + } + @Override protected void minXSearch() { if (isBufferSortedAscendingByX()) { From a1c5e09fcb8bc1826e2b26c192a2f244b0d382e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 06:52:21 +0000 Subject: [PATCH 4/7] DEV-896 Keep Swing and console I/O out of the batched chart lock; harden snapshot indexing Review follow-ups on the batched chart locking in filterDataAndPlot: - Swing under the chart monitor: updateHrPanelIfVisible() is overridden downstream (Consensys PlotManagerPC -> setPnlHR -> setLblHRValue) to do JLabel setText/revalidate/repaint from the data thread. Calling it inside synchronized(chart) meant the data thread took chart -> Swing tree / RepaintManager locks while the EDT takes tree lock -> chart inside Chart2D.paintComponent(): a lock inversion. The props of each trace needing an HR update are now recorded under the monitor and updateHrPanelIfVisible() is called after it is released - same args, same number of calls (one per matching trace), same order. - Console I/O under the monitor: throwExceptionSignalNotFound() (which dumps the whole ObjectCluster per missing signal) and printSignalProps() are now also recorded and replayed after the monitor is released, so a blocking console write can no longer stall the EDT. Debug output content is unchanged: printSignalProps() gained an overload taking an already-read trace size, so the size is still sampled at the original point in the loop. - The deferral buffers are reused instance fields, not per-sample allocations, and are only touched while the mListofPropertiestoPlot monitor is held (which the method holds for the whole batch). They are cleared at the start of each batch so a throw out of the loop cannot leak entries. - Off-by-one: the trace bounds check was 'indexOfTrace > length' (pre-existing as '> mListofTraces.size()'); index == length is already out of bounds, so it is now '>='. - Snapshot staleness: the trace array is snapshotted before entering the chart monitor (to preserve the mListofTraces -> chart lock ordering), so a trace removed mid-sample can still be in it. Added a cheap null / getRenderer() guard. A full "still attached" check is not viable here: jchart2d 3.3.2's Chart2D.removeTrace() does not clear the trace renderer and Chart2D .getTraces() builds a fresh TreeSet per call. - The per-sample toArray() at the snapshot is left as is (documented): any reuse would need the mListofTraces monitor at exactly the point where not taking it is the whole reason for the snapshot. Lock ordering re-verified: nothing inside synchronized(chartMonitor) synchronizes on mListofTraces or calls into Swing. addPointsToTrace's chunked chart lock does neither either. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015q29AEJZbwa7aVYeAj61Py --- .../guiUtilities/plot/BasicPlotManagerPC.java | 113 ++++++++++++++++-- 1 file changed, 104 insertions(+), 9 deletions(-) diff --git a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java index bb689b93d..79cb7192f 100644 --- a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java +++ b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java @@ -1650,6 +1650,47 @@ public void addPointToTrace(ITrace2D trace, double xData, double yData){ * Bounds how long the batch can hold the chart lock so a large burst can't monopolise the EDT. */ private static final int POINT_BATCH_MAX = 256; + /** + * DEV-896: Work recorded while {@link #filterDataAndPlot(ObjectCluster)} holds the chart monitor + * and replayed once the monitor has been released. Two kinds of work must not run under that + * monitor: + *

      + *
    • Swing calls - {@code updateHrPanelIfVisible()} is overridden downstream (Consensys) to do + * {@code JLabel.setText/revalidate/repaint} from the data thread. That would take + * chart -> Swing tree/RepaintManager locks while the EDT takes tree lock -> chart inside + * {@code Chart2D.paintComponent()}: a lock inversion.
      • + *
    • Console I/O - {@code throwExceptionSignalNotFound()} dumps a whole ObjectCluster per + * missing signal and {@code printSignalProps()} prints per sample in debug mode; holding the + * chart monitor across a blocking {@code System.out} write stalls the EDT for the duration.
      • + *
    + * These buffers are reused rather than allocated per sample, and are only ever touched while the + * {@code mListofPropertiestoPlot} monitor is held (which {@code filterDataAndPlot} holds for the + * whole batch), so no extra synchronization is needed. They are cleared at the start of each + * batch, so an exception thrown out of the loop cannot leak entries into the next sample. + */ + private final List mDeferredSignalNotFoundTraceNames = new ArrayList(); + /** DEV-896: See {@link #mDeferredSignalNotFoundTraceNames}. Holds the {@code props} of each trace + * that needs an HR panel update, in the order the traces were processed. */ + private final List mDeferredHrUpdateProps = new ArrayList(); + /** DEV-896: See {@link #mDeferredSignalNotFoundTraceNames}. Only populated in debug mode. */ + private final List mDeferredSignalPrints = new ArrayList(); + + /** DEV-896: One deferred {@code printSignalProps()} call. The trace size is captured at the point + * in the sample where the print would originally have happened (i.e. before this trace's point is + * added) so the deferred debug output is identical to the output before the lock was batched. */ + private static final class DeferredSignalPrint { + final int mTraceSize; + final String[] mProps; + final double mXData; + final double mYData; + DeferredSignalPrint(int traceSize, String[] props, double xData, double yData){ + mTraceSize = traceSize; + mProps = props; + mXData = xData; + mYData = yData; + } + } + /** * DEV-896: Batch variant of {@link #addPointToTrace(ITrace2D, double, double)}. Adds many * points to a single trace while acquiring the chart monitor once per (bounded) chunk rather @@ -1838,11 +1879,19 @@ private double getXDataForPlotting(String shimmerName, ObjectCluster ojc, int in } protected void printSignalProps(ObjectCluster ojc, ITrace2D currentTrace, String[] props, double xData, double yData){ + if(mIsDebugMode && currentTrace!=null){ + printSignalProps(ojc, currentTrace.getSize(), props, xData, yData); + } + } + + /** DEV-896: Variant taking an already-read trace size, so the caller can read the size while it + * holds the chart monitor but do the (blocking) console write after releasing it. */ + protected void printSignalProps(ObjectCluster ojc, int traceSize, String[] props, double xData, double yData){ if(mIsDebugMode){ utilShimmer.consolePrintErrLn( "ChartName:" + mChart.getName() - + "\tShimmerName:" + ojc.getShimmerName() - + "\ttrace size:" + currentTrace.getSize() + "." + + "\tShimmerName:" + ojc.getShimmerName() + + "\ttrace size:" + traceSize + "." + "\tprops1:" + props[1] + "." + "\tprops2:" + props[2] + "." + "\tx-value:" + xData + "." @@ -2013,8 +2062,17 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { //while calling chart-locking trace mutators (removeAllPoints/setMaxSize), i.e. //mListofTraces -> chart. Touching mListofTraces while holding the chart monitor here //would be the reverse order and a real deadlock cycle. + //The per-sample toArray() allocation is deliberate: reusing a cached array would need + //the mListofTraces monitor (or a copy under it) at exactly the point where taking that + //monitor is what we are avoiding, so there is no trivially safe reuse here. ITrace2D[] tracesSnapshot = mListofTraces.toArray(new ITrace2D[0]); Object chartMonitor = (mChart != null) ? (Object)mChart : (Object)mListofPropertiestoPlot; + //DEV-896: nothing inside the chart monitor below may call Swing or do console I/O + //(see mDeferredSignalNotFoundTraceNames) - such work is recorded here and replayed + //after the monitor is released. + mDeferredSignalNotFoundTraceNames.clear(); + mDeferredHrUpdateProps.clear(); + mDeferredSignalPrints.clear(); synchronized(chartMonitor){ while (entries.hasNext()) { String[] props = entries.next(); @@ -2033,7 +2091,7 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { FormatCluster f = ObjectCluster.returnFormatCluster(ojc.getCollectionOfFormatClusters(props[1]), props[2]); if(f == null){ indexOfTrace++; - throwExceptionSignalNotFound(traceName, ojc); + mDeferredSignalNotFoundTraceNames.add(traceName); //DEV-896: printed after the chart monitor is released continue; } @@ -2061,7 +2119,7 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { FormatCluster f = ObjectCluster.returnFormatCluster(ojc.getCollectionOfFormatClusters(props[1]), props[2]); if(f == null){ indexOfTrace++; - throwExceptionSignalNotFound(traceName, ojc); + mDeferredSignalNotFoundTraceNames.add(traceName); //DEV-896: printed after the chart monitor is released continue; } @@ -2074,18 +2132,37 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { continue; } - if (indexOfTrace>tracesSnapshot.length){ + //DEV-896: was '>' (pre-existing off-by-one against mListofTraces.size()); + //indexOfTrace == length is already out of bounds. + if (indexOfTrace>=tracesSnapshot.length){ throw new Exception("Trace does not exist: (" + traceName + ")"); } - ITrace2D currentTrace = tracesSnapshot[indexOfTrace]; + ITrace2D currentTrace = tracesSnapshot[indexOfTrace]; //utilShimmer.consolePrintErrLn(currentTrace.getMaxY()); + //DEV-896: the snapshot was taken before the chart monitor was entered, so a + //trace removed mid-sample can still be in it. Cheap staleness guard: a trace + //that has no renderer was never added to (or was never given) a chart, so + //adding points to it would only grow a buffer nothing paints. Note jchart2d + //3.3.2's Chart2D.removeTrace() does not clear the trace's renderer, and the + //only full "still attached" check, Chart2D.getTraces(), builds a fresh TreeSet + //on every call - far too expensive for this per-sample path. + if (currentTrace==null || currentTrace.getRenderer()==null){ + indexOfTrace++; + continue; + } + mCurrentXValue = xData; - printSignalProps(ojc, currentTrace, props, xData, yData); + //DEV-896: record instead of printing/updating Swing here - see + //mDeferredSignalNotFoundTraceNames. The trace size is read now so the deferred + //debug line matches what it printed before batching. + if(mIsDebugMode){ + mDeferredSignalPrints.add(new DeferredSignalPrint(currentTrace.getSize(), props, xData, yData)); + } + + mDeferredHrUpdateProps.add(props); - updateHrPanelIfVisible(props, ojc); - Double halfWindowSize = mMapofHalfWindowSize.get(traceName); if (halfWindowSize!=null){ if(addDummyPointToFillTraceIfRequired(currentTrace, xData-halfWindowSize)) { @@ -2131,6 +2208,24 @@ else if(isXAxisFrequency){ indexOfTrace++; } } //DEV-896: release the chart monitor once the whole multi-trace update is done + + //DEV-896: replay the work that must not run under the chart monitor. Order is + //preserved within each kind of work; the three kinds are independent of each other + //(two console dumps and a GUI label update). + for(int i=0; i Date: Fri, 4 Sep 2026 07:11:59 +0000 Subject: [PATCH 5/7] DEV-896 Replay deferred work on throw, bound per-sample lock hold, tidy fast-path edge cases Round-2 review follow-ups. - Deferred work was dropped on the "Trace does not exist" throw: that throw is inside synchronized(chartMonitor) while the replay ran after the block, so the HR updates and console dumps already recorded for earlier traces in the sample were lost (they used to run inline, and the downstream Consensys HR override counts calls, so a dropped call is observable). The loop's exception is now stashed, the replay runs, and the exception is rethrown afterwards. Replay therefore happens on every exit path. - Bounded the chart-monitor hold. Holding it across every trace of a sample made the worst-case EDT wait N_traces x O(buffer), because Trace2DLtd.addPointInternal still runs a full minYSearch/maxYSearch whenever the evicted point held the Y extreme - which for monotone or drifting Y channels (battery, temperature, GSR baseline, counters) is essentially every sample. The monitor is now released and re-acquired every TRACE_BATCH_MAX (8) traces, alongside the existing POINT_BATCH_MAX. The trace snapshot is still taken once, before the first acquisition, so the mListofTraces -> chart ordering is unchanged. Trace2DLtdMonotonicX's javadoc no longer implies the Y rescan is a non-issue; it now says Y is left to the superclass and that monotone Y channels still pay it. - Trace2DLtdMonotonicX: the O(1) X fast path returned before stock minXSearch/maxXSearch's trailing expandMinXErrorBarBounds/ expandMaxXErrorBarBounds, so an installed error bar policy would have been ignored. Both overrides now delegate wholly to the superclass when getErrorBarPolicies() is non-empty, and the "behaves identically to Trace2DLtd" wording is replaced by a description of that opt-out. - Dropped the getRenderer()==null snapshot-staleness guard. It could not detect staleness (Chart2D.removeTrace does not clear the renderer) and what it did catch was a never-attached trace, silently swallowing jchart2d's IllegalStateException and skipping the mCurrentXValue update. Only a plain null-check on the trace remains, with an accurate comment. - Replay now uses a single ordered list of DeferredPlotAction records instead of one list per kind, so the original per-trace interleaving is preserved - the two console kinds share System.out, so grouping by kind reordered debug output. The stale "the three kinds are independent" comment is gone. - The deferral buffers are method-local again. As instance fields they relied on mListofPropertiestoPlot being a stable monitor, but it is a public non-final field that AbstractPlotManager's constructors reassign. The per-sample allocation is dwarfed by the mListofTraces.toArray() snapshot the batch already needs, and the "reused rather than allocated per sample" comment is corrected. - HR recording stays unconditional: no HR-enabled flag exists in the base class (only the Consensys override knows mIsHRVisible), and that override counts calls, so it must stay one recorded action per matching trace. The comment now says so. Re-verified: nothing inside synchronized(chartMonitor) touches mListofTraces or calls Swing, the snapshot precedes the first acquisition, and the replay runs on all exit paths. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015q29AEJZbwa7aVYeAj61Py --- .../guiUtilities/plot/BasicPlotManagerPC.java | 191 ++++++++++++------ .../plot/Trace2DLtdMonotonicX.java | 30 ++- 2 files changed, 148 insertions(+), 73 deletions(-) diff --git a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java index 79cb7192f..9392943c3 100644 --- a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java +++ b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java @@ -1650,10 +1650,20 @@ public void addPointToTrace(ITrace2D trace, double xData, double yData){ * Bounds how long the batch can hold the chart lock so a large burst can't monopolise the EDT. */ private static final int POINT_BATCH_MAX = 256; + /** DEV-896: Max traces processed per single chart-monitor acquisition in + * {@link #filterDataAndPlot(ObjectCluster)}. Holding the monitor across every trace of a sample + * would let the worst-case EDT wait grow with the trace count: {@code Trace2DLtd.addPointInternal} + * still runs an O(buffer) {@code minYSearch/maxYSearch} whenever the evicted point held the Y + * extreme, which for a monotone or steadily drifting Y channel (battery, temperature, GSR + * baseline, sample counters) is essentially every sample. Re-acquiring every + * {@code TRACE_BATCH_MAX} traces keeps the churn saving while capping one hold at a constant + * number of those rescans. */ + private static final int TRACE_BATCH_MAX = 8; + /** - * DEV-896: Work recorded while {@link #filterDataAndPlot(ObjectCluster)} holds the chart monitor - * and replayed once the monitor has been released. Two kinds of work must not run under that - * monitor: + * DEV-896: A unit of work recorded while {@link #filterDataAndPlot(ObjectCluster)} holds the + * chart monitor and replayed, in the order recorded, once the monitor has been released. Two + * kinds of work must not run under that monitor: *
      *
    • Swing calls - {@code updateHrPanelIfVisible()} is overridden downstream (Consensys) to do * {@code JLabel.setText/revalidate/repaint} from the data thread. That would take @@ -1663,32 +1673,71 @@ public void addPointToTrace(ITrace2D trace, double xData, double yData){ * missing signal and {@code printSignalProps()} prints per sample in debug mode; holding the * chart monitor across a blocking {@code System.out} write stalls the EDT for the duration.
      • *
    - * These buffers are reused rather than allocated per sample, and are only ever touched while the - * {@code mListofPropertiestoPlot} monitor is held (which {@code filterDataAndPlot} holds for the - * whole batch), so no extra synchronization is needed. They are cleared at the start of each - * batch, so an exception thrown out of the loop cannot leak entries into the next sample. + * A single ordered list is used (rather than one list per kind) so the replay preserves the + * original per-trace interleaving - the two console kinds share {@code System.out}, so grouping + * by kind would reorder the debug output. The list and its entries are allocated per sample, + * which is dwarfed by the per-sample {@code mListofTraces.toArray()} snapshot the batch already + * needs; they are deliberately not shared instance state, because the only monitor that would + * make sharing safe ({@code mListofPropertiestoPlot}) is a public non-final field that + * {@code AbstractPlotManager}'s constructors reassign. */ - private final List mDeferredSignalNotFoundTraceNames = new ArrayList(); - /** DEV-896: See {@link #mDeferredSignalNotFoundTraceNames}. Holds the {@code props} of each trace - * that needs an HR panel update, in the order the traces were processed. */ - private final List mDeferredHrUpdateProps = new ArrayList(); - /** DEV-896: See {@link #mDeferredSignalNotFoundTraceNames}. Only populated in debug mode. */ - private final List mDeferredSignalPrints = new ArrayList(); - - /** DEV-896: One deferred {@code printSignalProps()} call. The trace size is captured at the point - * in the sample where the print would originally have happened (i.e. before this trace's point is - * added) so the deferred debug output is identical to the output before the lock was batched. */ - private static final class DeferredSignalPrint { - final int mTraceSize; + private static final class DeferredPlotAction { + static final int KIND_SIGNAL_NOT_FOUND = 0; + static final int KIND_PRINT_SIGNAL_PROPS = 1; + static final int KIND_UPDATE_HR_PANEL = 2; + + final int mKind; + final String mTraceName; final String[] mProps; + /** Read while the chart monitor is held, so the deferred debug line reports the same trace + * size it reported before the print was moved out of the lock. */ + final int mTraceSize; final double mXData; final double mYData; - DeferredSignalPrint(int traceSize, String[] props, double xData, double yData){ - mTraceSize = traceSize; + + private DeferredPlotAction(int kind, String traceName, String[] props, int traceSize, double xData, double yData){ + mKind = kind; + mTraceName = traceName; mProps = props; + mTraceSize = traceSize; mXData = xData; mYData = yData; } + + static DeferredPlotAction signalNotFound(String traceName){ + return new DeferredPlotAction(KIND_SIGNAL_NOT_FOUND, traceName, null, 0, 0, 0); + } + + static DeferredPlotAction printSignalProps(int traceSize, String[] props, double xData, double yData){ + return new DeferredPlotAction(KIND_PRINT_SIGNAL_PROPS, null, props, traceSize, xData, yData); + } + + static DeferredPlotAction updateHrPanel(String[] props){ + return new DeferredPlotAction(KIND_UPDATE_HR_PANEL, null, props, 0, 0, 0); + } + } + + /** DEV-896: Replays the work recorded by {@link #filterDataAndPlot(ObjectCluster)} while it held + * the chart monitor. Must be called on every exit path from the batched loop, including the + * "Trace does not exist" throw, because before the lock was batched this work ran inline (the + * downstream HR panel keeps a per-call counter, so a dropped call is observable). */ + private void replayDeferredPlotActions(List deferredActions, ObjectCluster ojc) throws Exception { + for(int i=0; i chart. Touching mListofTraces while holding the chart monitor here - //would be the reverse order and a real deadlock cycle. + //IMPORTANT (lock ordering): snapshot mListofTraces BEFORE the first chart-monitor + //acquisition, and keep using that one snapshot for the whole sample. Other threads + //(e.g. clearAllDataBuffer, trace resizing) hold the mListofTraces monitor while calling + //chart-locking trace mutators (removeAllPoints/setMaxSize), i.e. mListofTraces -> chart. + //Touching mListofTraces while holding the chart monitor here would be the reverse order + //and a real deadlock cycle. //The per-sample toArray() allocation is deliberate: reusing a cached array would need //the mListofTraces monitor (or a copy under it) at exactly the point where taking that //monitor is what we are avoiding, so there is no trivially safe reuse here. ITrace2D[] tracesSnapshot = mListofTraces.toArray(new ITrace2D[0]); Object chartMonitor = (mChart != null) ? (Object)mChart : (Object)mListofPropertiestoPlot; - //DEV-896: nothing inside the chart monitor below may call Swing or do console I/O - //(see mDeferredSignalNotFoundTraceNames) - such work is recorded here and replayed - //after the monitor is released. - mDeferredSignalNotFoundTraceNames.clear(); - mDeferredHrUpdateProps.clear(); - mDeferredSignalPrints.clear(); - synchronized(chartMonitor){ + //DEV-896: nothing inside the chart monitor below may call Swing or do console I/O - + //such work is recorded here and replayed afterwards (see DeferredPlotAction). + List deferredActions = new ArrayList(); + //DEV-896: stash rather than propagate, so the deferred work still gets replayed on the + //"Trace does not exist" path (it used to run inline, before the batching). + Exception pendingException = null; + try { while (entries.hasNext()) { + synchronized(chartMonitor){ + for(int tracesThisBatch=0; tracesThisBatchIf X ever arrives out of order (e.g. plot re-fed on rewind / replay / device reset) * the run is reset and we fall back to the superclass scans until enough monotonic * samples have refilled the buffer, so the displayed range is always exact. - *
  • Y is left entirely to the superclass. For random Y the oldest point holds the Y - * extreme only ~2/size of the time, so the Y rescan cost is already amortised O(1); - * optimising it is unnecessary and would risk the displayed Y auto-scale.
    • + *
  • Y is left entirely to the superclass, so the Y bounds are exactly the stock ones and no + * Y work is saved here. For random-walk Y the oldest point holds the Y extreme only + * ~2/size of the time, so that rescan is already amortised O(1). A monotone or steadily + * drifting Y channel (battery, temperature, GSR baseline, sample counters) is the bad case + * and still pays the superclass O(buffer) {@code minYSearch()}/{@code maxYSearch()} on + * essentially every sample; only the X half of the problem is fixed here. That is why the + * caller also bounds how many traces one chart-monitor acquisition covers.
    • *
  • {@code setMaxSize(int)} is {@code final} in {@code Trace2DLtd} so it cannot be * overridden, but no reset hook is needed: {@link #isBufferSortedAscendingByX()} reads * the live {@code m_buffer.size()} each call. Growing leaves the element count and @@ -46,9 +50,12 @@ * keeps the buffer sorted.
    • * * - *

    Externally this class behaves identically to {@code Trace2DLtd} (same bounds, same - * property-change events, same {@code setMaxSize} semantics); it only removes the redundant - * O(n) X rescans. It extends {@code Trace2DLtd} so existing + *

    Externally this class reports the same bounds, property-change events and + * {@code setMaxSize} semantics as {@code Trace2DLtd}; it only removes the redundant O(n) X + * rescans. The one behavioural difference is a deliberate opt-out rather than a divergence: if any + * error bar policy is installed on the trace, both X searches delegate wholly to the superclass, + * because stock {@code minXSearch()}/{@code maxXSearch()} finish by folding the error bar extents + * into the bounds and the O(1) path has no equivalent. It extends {@code Trace2DLtd} so existing * {@code ((Trace2DLtd)trace).setMaxSize(...)} / {@code .iterator()} casts keep working.

    */ public class Trace2DLtdMonotonicX extends Trace2DLtd { @@ -142,6 +149,12 @@ public void firePointChanged(final ITracePoint2D changed, final int state) { @Override protected void minXSearch() { + //Stock minXSearch() ends with expandMinXErrorBarBounds(); the O(1) path cannot reproduce + //that, so with any error bar policy installed defer entirely to the superclass. + if (!getErrorBarPolicies().isEmpty()) { + super.minXSearch(); + return; + } if (isBufferSortedAscendingByX()) { try { // Oldest element holds the smallest X when the buffer is sorted ascending. @@ -156,6 +169,11 @@ protected void minXSearch() { @Override protected void maxXSearch() { + //See minXSearch(): stock maxXSearch() ends with expandMaxXErrorBarBounds(). + if (!getErrorBarPolicies().isEmpty()) { + super.maxXSearch(); + return; + } if (isBufferSortedAscendingByX()) { try { // Youngest element holds the largest X when the buffer is sorted ascending. From 28f1f04587b60b4fbb364e755af6a589db1404a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 07:25:47 +0000 Subject: [PATCH 6/7] DEV-896 Address review comments: null-safe replay, skip HR deferral when not overridden - filterDataAndPlot() supports mChart == null (the chart-monitor fallback), and the deferred replay can run throwExceptionSignalNotFound() or printSignalProps() in that state, both of which printed mChart.getName(). Both now go through getChartNameForPrinting(), which yields "" when no chart is set. The third replayed action, updateHrPanelIfVisible(), is a no-op in this class and the downstream override touches only the props, the ObjectCluster and its own labels, so it needs nothing. - The per-trace deferred HR action is now only recorded when the runtime class actually overrides updateHrPanelIfVisible(); in this base class it is a no-op, so the record and its allocation were pure waste. Detected once per instance by walking the class hierarchy with getDeclaredMethod() (the method is protected, so getMethod() would not see it) from the runtime class up to BasicPlotManagerPC, cached in a final field. Any unexpected reflective failure defaults to true, i.e. to the previous unconditional behaviour. Verified: the base class resolves to false and an overriding subclass to true, so the Consensys PlotManagerPC path keeps exactly one call per matching trace. - Documented why the mListofTraces snapshot needs no explicit synchronized(mListofTraces): the field is a Collections.synchronizedList, so toArray() already copies under that list's own mutex, and the index alignment with mListofPropertiestoPlot that actually matters is protected by the mListofPropertiestoPlot monitor held for the whole sample. No code change. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015q29AEJZbwa7aVYeAj61Py --- .../guiUtilities/plot/BasicPlotManagerPC.java | 67 +++++++++++++++++-- 1 file changed, 60 insertions(+), 7 deletions(-) diff --git a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java index 9392943c3..97013a10d 100644 --- a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java +++ b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java @@ -1717,6 +1717,47 @@ static DeferredPlotAction updateHrPanel(String[] props){ } } + /** DEV-896: {@code mChart} is optional - {@link #filterDataAndPlot(ObjectCluster)} falls back to + * another monitor when no chart is set yet - and the deferred replay can run + * {@code throwExceptionSignalNotFound()} / {@code printSignalProps()} in that state, so neither + * may dereference {@code mChart} directly. */ + private String getChartNameForPrinting(){ + return (mChart!=null)? mChart.getName() : ""; + } + + /** + * DEV-896: True when the runtime class overrides + * {@link #updateHrPanelIfVisible(String[], ObjectCluster)}. In this base class that method is a + * no-op, so there is no point recording (and allocating) a deferred HR action per trace per + * sample for plots that will never do anything with it; only the downstream override (Consensys + * {@code PlotManagerPC}) needs them, and it counts its calls, so when it IS present every + * matching trace must still produce exactly one call. + * + *

    Resolved once per instance rather than per sample. The method is {@code protected}, so + * {@code getMethod()} would not see it - the class hierarchy is walked with + * {@code getDeclaredMethod()} from the runtime class up to (but excluding) this class instead. + * Anything unexpected from the reflective lookup defaults to {@code true}, i.e. to the previous + * unconditional behaviour, so a hardened SecurityManager can only cost the allocation, never + * suppress an HR update.

    + */ + private final boolean mIsHrPanelUpdateOverridden = isHrPanelUpdateOverridden(getClass()); + + private static boolean isHrPanelUpdateOverridden(Class runtimeClass){ + try { + for(Class c = runtimeClass; c!=null && c!=BasicPlotManagerPC.class; c = c.getSuperclass()){ + try { + c.getDeclaredMethod("updateHrPanelIfVisible", String[].class, ObjectCluster.class); + return true; + } catch (NoSuchMethodException e) { + //Not declared at this level, keep walking up towards BasicPlotManagerPC. + } + } + return false; + } catch (Throwable t) { + return true; //Safe default: behave exactly as before the optimisation. + } + } + /** DEV-896: Replays the work recorded by {@link #filterDataAndPlot(ObjectCluster)} while it held * the chart monitor. Must be called on every exit path from the batched loop, including the * "Trace does not exist" throw, because before the lock was batched this work ran inline (the @@ -1886,7 +1927,7 @@ private void throwExceptionSignalNotFound(String[] props, ObjectCluster ojc) thr } private void throwExceptionSignalNotFound(String traceName, ObjectCluster ojc) throws Exception { - utilShimmer.consolePrintLn("mChart.getName(): " +mChart.getName()); + utilShimmer.consolePrintLn("mChart.getName(): " +getChartNameForPrinting()); if(ojc!=null) { ojc.consolePrintChannelsAndDataSingleLine(); } @@ -1938,7 +1979,7 @@ protected void printSignalProps(ObjectCluster ojc, ITrace2D currentTrace, String protected void printSignalProps(ObjectCluster ojc, int traceSize, String[] props, double xData, double yData){ if(mIsDebugMode){ utilShimmer.consolePrintErrLn( - "ChartName:" + mChart.getName() + "ChartName:" + getChartNameForPrinting() + "\tShimmerName:" + ojc.getShimmerName() + "\ttrace size:" + traceSize + "." + "\tprops1:" + props[1] + "." @@ -2116,6 +2157,15 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { //The per-sample toArray() allocation is deliberate: reusing a cached array would need //the mListofTraces monitor (or a copy under it) at exactly the point where taking that //monitor is what we are avoiding, so there is no trivially safe reuse here. + //No explicit synchronized(mListofTraces) is needed for the snapshot itself either: + //mListofTraces is a Collections.synchronizedList, so toArray() already copies under + //that list's own mutex and cannot observe a half-applied structural change. Its index + //alignment with mListofPropertiestoPlot is what actually matters here, and that is + //protected by the mListofPropertiestoPlot monitor this method holds for the whole + //sample: removeSignal()/removeSignalInternal() mutate both lists under it. The one + //exception is removeAllSignals(), which holds neither - but it also clears + //mListofPropertiestoPlot underneath this method's live iterator, a pre-existing hazard + //that predates and is independent of this batching. ITrace2D[] tracesSnapshot = mListofTraces.toArray(new ITrace2D[0]); Object chartMonitor = (mChart != null) ? (Object)mChart : (Object)mListofPropertiestoPlot; //DEV-896: nothing inside the chart monitor below may call Swing or do console I/O - @@ -2219,11 +2269,14 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { deferredActions.add(DeferredPlotAction.printSignalProps(currentTrace.getSize(), props, xData, yData)); } - //Recorded unconditionally: whether an HR panel is actually visible is known - //only to the downstream (Consensys) updateHrPanelIfVisible() override, and that - //override counts calls, so this must stay one recorded action per matching - //trace. In the base class the replayed call is a no-op. - deferredActions.add(DeferredPlotAction.updateHrPanel(props)); + //Recorded once per matching trace whenever the runtime class actually overrides + //updateHrPanelIfVisible(): whether a panel is currently visible is known only + //to that override, and it counts its calls, so no further filtering is safe. + //When it is not overridden the replayed call would be a no-op, so skip the + //record (and its allocation) entirely - see mIsHrPanelUpdateOverridden. + if(mIsHrPanelUpdateOverridden){ + deferredActions.add(DeferredPlotAction.updateHrPanel(props)); + } Double halfWindowSize = mMapofHalfWindowSize.get(traceName); if (halfWindowSize!=null){ From 7a6df50f222b2bf50dd212490c58ce7e57acbbea Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 07:31:33 +0000 Subject: [PATCH 7/7] DEV-896 Lazily allocate the deferred-action list on the plot hot path filterDataAndPlot() allocated an ArrayList for the deferred chart-monitor work on every sample, even though on the typical sample nothing is recorded into it (no debug mode, no missing signal, and no updateHrPanelIfVisible() override). The local now starts null and is created on first record, via a small recordDeferredPlotAction() helper that returns the list to assign back; replayDeferredPlotActions() treats null as "nothing recorded" and returns immediately. Semantics are unchanged: actions are still appended in the order they are recorded and replayed in that same order, and the replay still runs on every exit path from the batched loop, the "Trace does not exist" throw included. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015q29AEJZbwa7aVYeAj61Py --- .../guiUtilities/plot/BasicPlotManagerPC.java | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java index 97013a10d..2f539f5fd 100644 --- a/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java +++ b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/BasicPlotManagerPC.java @@ -1758,11 +1758,27 @@ private static boolean isHrPanelUpdateOverridden(Class runtimeClass){ } } + /** DEV-896: Appends one action to the (lazily created) deferral list and returns the list to + * assign back, so the common case - no debug mode, no missing signal, no HR override - allocates + * nothing at all on this per-sample path. Ordering is unaffected: actions are still appended in + * the order they are recorded. */ + private static List recordDeferredPlotAction(List deferredActions, DeferredPlotAction action){ + if(deferredActions == null){ + deferredActions = new ArrayList(); + } + deferredActions.add(action); + return deferredActions; + } + /** DEV-896: Replays the work recorded by {@link #filterDataAndPlot(ObjectCluster)} while it held * the chart monitor. Must be called on every exit path from the batched loop, including the * "Trace does not exist" throw, because before the lock was batched this work ran inline (the - * downstream HR panel keeps a per-call counter, so a dropped call is observable). */ + * downstream HR panel keeps a per-call counter, so a dropped call is observable). A {@code null} + * list means nothing was recorded (see {@link #recordDeferredPlotAction}) and is a no-op. */ private void replayDeferredPlotActions(List deferredActions, ObjectCluster ojc) throws Exception { + if(deferredActions == null){ + return; + } for(int i=0; i deferredActions = new ArrayList(); + //Left null until something is actually recorded: on the typical sample (no debug + //mode, no missing signal, no HR override) nothing is, so this per-sample path + //allocates no list at all. See recordDeferredPlotAction(). + List deferredActions = null; //DEV-896: stash rather than propagate, so the deferred work still gets replayed on the //"Trace does not exist" path (it used to run inline, before the batching). Exception pendingException = null; @@ -2194,7 +2213,7 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { FormatCluster f = ObjectCluster.returnFormatCluster(ojc.getCollectionOfFormatClusters(props[1]), props[2]); if(f == null){ indexOfTrace++; - deferredActions.add(DeferredPlotAction.signalNotFound(traceName)); //DEV-896: printed after the chart monitor is released + deferredActions = recordDeferredPlotAction(deferredActions, DeferredPlotAction.signalNotFound(traceName)); //DEV-896: printed after the chart monitor is released continue; } @@ -2222,7 +2241,7 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { FormatCluster f = ObjectCluster.returnFormatCluster(ojc.getCollectionOfFormatClusters(props[1]), props[2]); if(f == null){ indexOfTrace++; - deferredActions.add(DeferredPlotAction.signalNotFound(traceName)); //DEV-896: printed after the chart monitor is released + deferredActions = recordDeferredPlotAction(deferredActions, DeferredPlotAction.signalNotFound(traceName)); //DEV-896: printed after the chart monitor is released continue; } @@ -2266,7 +2285,7 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { //The trace size is read now, under the monitor, so the deferred debug line //matches what it printed before batching. if(mIsDebugMode){ - deferredActions.add(DeferredPlotAction.printSignalProps(currentTrace.getSize(), props, xData, yData)); + deferredActions = recordDeferredPlotAction(deferredActions, DeferredPlotAction.printSignalProps(currentTrace.getSize(), props, xData, yData)); } //Recorded once per matching trace whenever the runtime class actually overrides @@ -2275,7 +2294,7 @@ public void filterDataAndPlot(ObjectCluster ojc) throws Exception { //When it is not overridden the replayed call would be a no-op, so skip the //record (and its allocation) entirely - see mIsHrPanelUpdateOverridden. if(mIsHrPanelUpdateOverridden){ - deferredActions.add(DeferredPlotAction.updateHrPanel(props)); + deferredActions = recordDeferredPlotAction(deferredActions, DeferredPlotAction.updateHrPanel(props)); } Double halfWindowSize = mMapofHalfWindowSize.get(traceName);