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..2f539f5fd 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); @@ -1645,7 +1645,203 @@ 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: 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: 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: + * + * 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 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; + + 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: {@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: 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). 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; istartBin){ 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 per group of TRACE_BATCH_MAX traces 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 point 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 monitor is released and re-acquired between groups so one hold stays bounded by a + //constant, not by the trace count (see TRACE_BATCH_MAX). + //Falls back to the already-held mListofPropertiestoPlot monitor if no chart is set yet. + //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. + //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 - + //such work is recorded here and replayed afterwards (see DeferredPlotAction). + //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; + try { while (entries.hasNext()) { + synchronized(chartMonitor){ + for(int tracesThisBatch=0; tracesThisBatchmListofTraces.size()){ + //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 = mListofTraces.get(indexOfTrace); + ITrace2D currentTrace = tracesSnapshot[indexOfTrace]; //utilShimmer.consolePrintErrLn(currentTrace.getMaxY()); + //DEV-896: defensive null check only. The snapshot is taken before the first + //chart-monitor acquisition, so in principle a trace removed mid-sample could + //still be in it, but there is no cheap way to detect that: jchart2d 3.3.2's + //Chart2D.removeTrace() does not clear the trace's renderer, and the only real + //"still attached" check, Chart2D.getTraces(), builds a fresh TreeSet per call. + //In practice removeSignal()/removeSignalInternal() mutate mListofTraces under + //the mListofPropertiestoPlot monitor this method holds for the whole sample, so + //a stale entry cannot appear via them; a stale entry from any other path just + //receives points into a buffer nothing paints, as it did before the batching. + //Do NOT filter on getRenderer()==null here: that only catches a trace that was + //never attached to a chart, and silently swallowing the IllegalStateException + //jchart2d raises for that would also skip the mCurrentXValue update below. + if (currentTrace==null){ + indexOfTrace++; + continue; + } + mCurrentXValue = xData; - printSignalProps(ojc, currentTrace, props, xData, yData); + //DEV-896: record instead of printing/updating Swing here - see DeferredPlotAction. + //The trace size is read now, under the monitor, so the deferred debug line + //matches what it printed before batching. + if(mIsDebugMode){ + deferredActions = recordDeferredPlotAction(deferredActions, DeferredPlotAction.printSignalProps(currentTrace.getSize(), props, xData, yData)); + } + + //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 = recordDeferredPlotAction(deferredActions, DeferredPlotAction.updateHrPanel(props)); + } - updateHrPanelIfVisible(props, ojc); - Double halfWindowSize = mMapofHalfWindowSize.get(traceName); if (halfWindowSize!=null){ if(addDummyPointToFillTraceIfRequired(currentTrace, xData-halfWindowSize)) { @@ -2067,6 +2341,20 @@ else if(isXAxisFrequency){ } indexOfTrace++; } + } //DEV-896: release the chart monitor between groups of TRACE_BATCH_MAX traces + } //while(entries.hasNext()) + } catch (Exception e) { + pendingException = e; + } + + //DEV-896: replay, in the recorded order, the work that must not run under the chart + //monitor. This runs on every exit path from the loop above, the "Trace does not exist" + //throw included, because before the batching it ran inline per trace. + replayDeferredPlotActions(deferredActions, ojc); + if(pendingException != null){ + throw pendingException; + } + if(isDummyPointAddedToFillTrace) { isFirstPointOnFillTrace = false; } 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..b025245b5 --- /dev/null +++ b/ShimmerDriverPC/src/main/java/com/shimmerresearch/guiUtilities/plot/Trace2DLtdMonotonicX.java @@ -0,0 +1,188 @@ +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:

+ *
    + *
  • We only take the O(1) path when the buffer is guaranteed sorted ascending by X. + * That guarantee holds iff the most recent {@code size()} insertions all had + * non-decreasing X, because a ring buffer holds exactly the most recent + * {@code size()} insertions. We track the length of the current run of consecutive + * non-decreasing-X insertions ({@link #mAscendingRunLength}); when it is at least the + * current buffer element count the whole buffer content is that non-decreasing suffix, + * hence sorted.
  • + *
  • If 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, 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 + * 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 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 { + + /** + * 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 volatile 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(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) { + 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); + } + + /** + * 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() { + //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. + 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() { + //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. + m_maxX = m_buffer.getYoungest().getX(); + return; + } catch (RuntimeException e) { + // Fall back to the safe scan. + } + } + super.maxXSearch(); + } +}