Merge pull request #500 from tiresiasfromthebai/perf-scatter-array-visualizer

perf: speed up the Array Visualizer scatter phase-portrait

src/SeerArrayVisualizerWidget.cpp

@@ -28,11 +28,18 @@
 // Qt Charts 6.x workaround: scatter marker items can be (re)created without
 // inheriting the series' pen/brush, leaving near-invisible ~1px markers.
 // Small arrays usually render fine; large ones (e.g. 4000 points) reliably
-// degrade. The series-level state is correct in both cases (verified with an
-// instrumented build), so the style is lost at the marker-item level inside
-// Qt Charts. Because the series setters are guarded (a call with an unchanged
-// value is a no-op), we force two *actual* value transitions so the style is
-// pushed down to the marker items unconditionally.
+// degrade. The series-level state is correct in both cases, so the style is
+// lost at the marker-item level inside Qt Charts. Re-applying the pen/brush
+// after the series is added pushes the style down to the marker items.
+//
+// Two passes suffice, not four. This runs after addSeries(), so the series
+// already carries the chart theme's pen/brush; setPen(NoPen) and setBrush()
+// are therefore real value transitions (no built-in Qt theme uses NoPen for
+// a scatter pen nor this exact colour for the brush) and are not swallowed
+// by the guarded setters. Even if one were a no-op, the other setter's
+// update re-pushes both pen and brush to every marker item. Two passes over
+// the (up to thousands of) marker items instead of four roughly halves this
+// step's cost, with no visible change to the markers.
 //
 static void redecorateScatterSeries (QXYSeries* series) {
 
@@ -42,8 +49,6 @@ static void redecorateScatterSeries (QXYSeries* series) {
         return;
     }
 
-    scatter->setPen(QPen(Qt::transparent));
-    scatter->setBrush(QBrush(Qt::transparent));
     scatter->setPen(QPen(Qt::NoPen));
     scatter->setBrush(QBrush(QColor(31, 119, 180)));
 }
@@ -164,6 +169,10 @@ void SeerArrayVisualizerWidget::setAVariableName (const QString& name) {
         return;
     }
 
+    // Coalesce this variable's setup (address reset + data/offset/stride) into
+    // a single table rebuild; the guard flushes it when it leaves scope.
+    SeerArrayWidget::BulkUpdate bulk(arrayTableWidget);
+
     setAVariableAddress("");
 
     // Clear old contents.
@@ -282,6 +291,10 @@ void SeerArrayVisualizerWidget::setBVariableName (const QString& name) {
         return;
     }
 
+    // Coalesce this variable's setup (address reset + data/offset/stride) into
+    // a single table rebuild; the guard flushes it when it leaves scope.
+    SeerArrayWidget::BulkUpdate bulk(arrayTableWidget);
+
     setBVariableAddress("");
 
     // Clear old contents.
@@ -540,6 +553,8 @@ void SeerArrayVisualizerWidget::handleText (const QString& text) {
 
                 // Give the byte array to the hex widget.
                 bool ok;
+                SeerArrayWidget::BulkUpdate bulk(arrayTableWidget);
+
                 arrayTableWidget->setAData(arrayTableWidget->aLabel(), new SeerArrayWidget::DataStorageArray(array));
 
                 if (aArrayOffsetLineEdit->text() != "") {
@@ -587,6 +602,8 @@ void SeerArrayVisualizerWidget::handleText (const QString& text) {
 
                 // Give the byte array to the hex widget.
                 bool ok;
+                SeerArrayWidget::BulkUpdate bulk(arrayTableWidget);
+
                 arrayTableWidget->setBData(arrayTableWidget->bLabel(), new SeerArrayWidget::DataStorageArray(array));
 
                 if (bArrayOffsetLineEdit->text() != "") {

src/SeerArrayWidget.cpp

@@ -33,6 +33,9 @@ SeerArrayWidget::SeerArrayWidget(QWidget* parent) : QTableWidget(parent) {
     _bAddressOffset  = 0;
     _bAddressStride  = 1;
 
+    _bulkUpdateDepth = 0;
+    _createPending   = false;
+
     setAAddressOffset(0);
     setAAddressStride(1);
 
@@ -314,6 +317,14 @@ void SeerArrayWidget::setBData(const QString& label, SeerArrayWidget::DataStorag
 
 void SeerArrayWidget::create () {
 
+    // Coalesce a burst of property changes: while a bulk update is active,
+    // defer the (expensive) table rebuild and just record that one is pending.
+    // endBulkUpdate() performs the single rebuild once the burst is complete.
+    if (_bulkUpdateDepth > 0) {
+        _createPending = true;
+        return;
+    }
+
     // Clear the table. We're going to recreate it.
     clear();
     setRowCount(0);
@@ -584,6 +595,32 @@ void SeerArrayWidget::create () {
     emit dataChanged();
 }
 
+void SeerArrayWidget::beginBulkUpdate () {
+
+    _bulkUpdateDepth++;
+}
+
+void SeerArrayWidget::endBulkUpdate () {
+
+    if (_bulkUpdateDepth > 0) {
+        _bulkUpdateDepth--;
+    }
+
+    // Once fully unwound, do the single deferred rebuild (if any setter fired).
+    if (_bulkUpdateDepth == 0 && _createPending) {
+        _createPending = false;
+        create();
+    }
+}
+
+SeerArrayWidget::BulkUpdate::BulkUpdate (SeerArrayWidget* widget) : _widget(widget) {
+    _widget->beginBulkUpdate();
+}
+
+SeerArrayWidget::BulkUpdate::~BulkUpdate () {
+    _widget->endBulkUpdate();
+}
+
 SeerArrayWidget::DataStorageArray::DataStorageArray(const QByteArray& arr) {
     _data = arr;
 }

src/SeerArrayWidget.h

@@ -46,6 +46,21 @@ class SeerArrayWidget: public QTableWidget {
 
         int                         elementsPerLine         () const;
 
+        // RAII guard: coalesces a burst of property changes into a single
+        // rebuild. While a guard is alive create() is deferred; the single
+        // rebuild happens when it goes out of scope. Nesting is supported.
+        // Prefer a guard over the private begin/end pair so the matching end
+        // is guaranteed even on early return or exception.
+        class BulkUpdate {
+            public:
+                explicit BulkUpdate     (SeerArrayWidget* widget);
+                               ~BulkUpdate ();
+                                BulkUpdate (const BulkUpdate&) = delete;
+                BulkUpdate&     operator= (const BulkUpdate&) = delete;
+            private:
+                SeerArrayWidget*        _widget;
+        };
+
         const QString&              aAxis                   () const;
         void                        setAAxis                (const QString& axis);
         const QString&              aLabel                  () const;
@@ -88,6 +103,11 @@ class SeerArrayWidget: public QTableWidget {
 
     private:
         void                        create                  ();
+        void                        beginBulkUpdate         ();
+        void                        endBulkUpdate           ();
+
+        int                         _bulkUpdateDepth;
+        bool                        _createPending;
 
         QString                     _aAxis;
         QString                     _aLabel;

tests/helloscatter/Makefile

@@ -0,0 +1,20 @@
+# This is the default target, which will be built when
+# you invoke make
+.PHONY: all
+all: helloscatter
+
+# This rule tells make how to build helloscatter from helloscatter.cpp
+helloscatter: helloscatter.cpp
+	g++ -g -o helloscatter helloscatter.cpp
+
+# This rule tells make to copy helloscatter to the binaries subdirectory,
+# creating it if necessary
+.PHONY: install
+install:
+	mkdir -p binaries
+	cp -p helloscatter binaries
+
+# This rule tells make to delete helloscatter
+.PHONY: clean
+clean:
+	rm -f helloscatter helloscatter.o

tests/helloscatter/helloscatter.cpp

@@ -0,0 +1,49 @@
+#include <cstdio>
+
+#define NPOINTS 4000
+
+double pos[NPOINTS];
+double vel[NPOINTS];
+
+//
+// Fill pos/vel with the trajectory of a damped harmonic oscillator
+// (semi-implicit Euler integration).
+//
+void integrate (double damping) {
+
+    double x  = 1.0;
+    double v  = 0.0;
+    double w  = 1.0;
+    double dt = 0.005;
+
+    for (int i=0; i<NPOINTS; i++) {
+        v += (-w*w*x - damping*v) * dt;
+        x += v * dt;
+
+        pos[i] = x;
+        vel[i] = v;
+    }
+}
+
+//
+// Exercises the Array Visualizer with a large two-array scatter plot.
+//
+// Open the Array Visualizer, enter 'pos' as array A and 'vel' as array B,
+// length 4000, select B's axis as 'Y', scatter mode, and check 'Auto'.
+// Then set a breakpoint on the printf below and 'Continue' a few times:
+// each stop recomputes the trajectory with a different damping and
+// refreshes the 4000-point phase portrait.
+//
+int main (void) {
+
+    for (int run=0; run<10; run++) {
+
+        double damping = 0.05 + 0.05*run;
+
+        integrate(damping);
+
+        printf("run %d: damping=%.2f  pos[end]=%f\n", run, damping, pos[NPOINTS-1]);
+    }
+
+    return 0;
+}