Aggressive splom perf #3057

etpinard · 2018-09-28T15:08:14Z

Completes the splom part of #2553 and checks off two O(n^2) items in #2555.

Each commit here is fairly self-contained. The most aggresive optimizations are in 17e30d2 (aka supplyDefaults-bypass) and 4792f08 (aka no-need-for-<rect.bg>-nor-subplot-<clipPath>) Note that commit 3574ece was required to not have to re-init the splom objects on editType: 'style' updates.

In terms of performance gains, using:

var Nvars = 50
var Nrows = 800
var dims = []

for(var i = 0; i < Nvars; i++) {
  dims.push({values: []})
  for(var j = 0; j < Nrows; j++) {
     dims[i].values.push(Math.random())
  }
}

var gd = document.getElementById('graph')
Plotly.purge(gd);

console.time('splom')
Plotly.newPlot(gd, [{
  type: 'splom',
  dimensions: dims,
  showupperhalf: false,
  diagonal: {visible: false}
  // totaling 1225 subplots
}], {width: 2000, height: 2000},
   {showAxisDragHandles: false, showAxisRangeEntryBoxes: false}
)
console.timeEnd('splom')

and comparing against v1.41.3 on Chrome 69 (which significantly slower than Chrome 66, by the way), we get gains of:

~200 ms on first render, thanks to the no-need-for-<rect.bg>-nor-subplot-<clipPath>
~200 ms on relayout(gd, 'xaxis.range', [/* */]), thanks to cc5f0ca and the supplyDefaults-bypass
huge improvements for restyle(gd, 'marker.color', /* scalar */): ~150ms vs 1000ms
huge improvements (but not good enough) for restyle(gd, 'marker.size', /* scalar */): ~400ms vs 1000ms
huge improvements (but not good enough) for restyle(gd, 'marker.color', '/* arrayOk */): ~800ms vs 2000ms

cc @alexcjohnson @antoinerg @archmoj

- scattergl 'text' now has the same behavior a scatter - splom 'text' is hover-only

... and out of the gd.calcdata[i][0].t stash, so that: - scene refs can be relinked properly on updates - scene refs can be destroyed properly when needed - move also visibileDims out of calcdata stash to fullData[i], for convenience.

... instead of 'style', for consistency. Other _module.style in the traces/ get called during Plots.style, something we don't want to do in regl traces, to avoid double-drawing.

... as opposed to once per splom trace dimensions.

- matrix.update() can be expansive (something we should look into), so try to call it the least number of times possible.

- by just calling 'Axes.doTicks' instead of full `doTicksRelayout` which includes drawing the title - this also fixes (and locks) a bug where large splom would draw twice (once in doTicksRelayout and once in the drawData subroutine).

... by passing Plots.supplyDefaults which can take more than 100ms for large splom traces (because of all those axes to coerce). - N.B. this commit applies to optimization to all 'axrange', I could make apply only to hasOnlyLargeSplom graphs if deemed too dangerous.

... cutting down ~200ms for 50 dims sploms - do not append <rect.bg> to DOM, when plot_bgcolor === papep_bgcolor - do not append useless <g.plot> layer to DOM - do not try to setup plot area clip paths under hasOnlyLargeSplom regime - loop over subplots ids, instead of a costly selectAll() + .each()

- using `_module.editStyle`, and not _module.style which would lead to double drawing from Plots.style. - in brief, for 'style' edits, we: + clear gl canvas + merge new convertMarkerStyle() into matrixOptions + call matrix.update and matrix.draw

- marker.size is hard to edit fast, as in general it can change the axis ranges

etpinard · 2018-09-28T15:20:00Z

Here are some benchmarks in the splom pipeline off this branch. At 50 dims and 800 rows, we clock:

supplyDefaults: 100-150 ms
doCalcdata: ~20 ms
drawFramework (where the <g.subplot> are added to the DOM): 200-300 ms
- room for improvement here!
lsInner (where subplot, bg, lines and clip paths are updated): 40-60 ms
drawAxes: 300-400 ms
initInteractions (where the drag layers are added to the DOM):
- ~100-150 ms (with showAxisDragHandles and showAxisRangeEntryBoxes false)
- ~1000 ms (default),
createREGL: ~300-400 ms on a 2000x2000px canvas
splom grid update/draw: ~150-200 ms,
- we could add dimensions[i].axis.showgrid: false to skip this
splom init (the createMatrix call): ~200 ms (not sure why)
splom update/draw: ~400-600 ms

which gives a total range of 1800 to 2500 ms.

alexcjohnson · 2018-09-29T00:03:23Z

src/traces/splom/base_plot.js

+            if(oldFullLayout._splomScenes) {
+                var scene = oldFullLayout._splomScenes[oldTrace.uid];
+                if(scene && scene.destroy) scene.destroy();
+                // must first set scene to null in order to get garbage collected


interesting... how did you figure this out?

Checking GPU memory consumption in the Chrome task manager (<shift>+<esc>) before and after calling Plotly.purge(gd).

Alright, I have to wonder

How general that observation is - does this apply across browsers/OS's? does it apply when the console is closed?

Assuming it does apply in some real-world situations, where else should we be doing the same thing? Would it be worthwhile making a Lib.delete(container, key) or something to encourage reuse?

src/traces/splom/base_plot.js

src/plot_api/plot_api.js

src/plot_api/subroutines.js

alexcjohnson · 2018-09-29T03:02:37Z

initInteractions (where the drag layers are added to the DOM):

~100-150 ms (with showAxisDragHandles and showAxisRangeEntryBoxes false)

~1000 ms (default),

Wow - time to experiment with a single drag element that registers many separate regions?

alexcjohnson · 2018-09-29T03:18:09Z

Fantastic work @etpinard - very impressive results!

Just two questions of safety #3057 (comment) #3057 (comment) and a couple of potential small improvements #3057 (comment) #3057 (comment)

The single drag element is already mentioned in #2555 and obviously should not be done here, that should be its own PR.

... but do draw <rect.bg> when either plot or paper bgcolor are semi-transparent.

src/plot_api/subroutines.js

- which (re)-draw scattergl, scatterpolargl, splom traces as well as splom regl grid line in one go, always in the correct order - packaging these gl draw calls in one subroutine is especially useful for drag and selections, where buffers of targeted traces/scene are updated, but *all* traces need to be redraw following clearGlCanvases

... no need for this now that we have redrawReglTraces

- no longer need _module.styleOnSelect! - selections on overlaid subplots now work! - remove clearViewport (for now), we could bring it back again to optimize selections on disjoint subplots.

test/jasmine/assets/custom_assertions.js

Add redrawReglTraces subroutine

alexcjohnson · 2018-10-02T19:32:45Z

Beautiful. 💃

.... and diagonal.visible

etpinard · 2018-10-03T19:39:52Z

Improvemens (e.g. no axis type clearance) over 078c086 in -> splom-perf...splom-axis-dim-match

This reverts commit 078c086.

... regardless of the partial visibilities. ie `trace.xaxes: ['x', 'x2', 'x3', 'x4']` even though we aren’t going to create an `xaxis4`. Should make it easier for users who don’t want to use the default axes but still want to be able to turn on/off the upper/lower/diag

test/jasmine/tests/splom_test.js

from empet's bug report: https://community.plot.ly/t/ticklabels-in-splom-with-diagonal-visible-false-are-translated/14196

alexcjohnson · 2018-10-04T18:17:55Z

src/traces/splom/defaults.js

+    // when lower half is omitted, or when just the diagonal is gone,
+    // override grid default to make sure axes remain on
+    // the left/bottom of the plot area
+    if(!showLower || (!showDiag && showUpper && showLower)) {


I could imagine splom always setting the defaults to 'bottom' and 'left'... but I guess in principle this way at least if you're in one of the situations that doesn't need this, you could make a bigger grid than the splom and put other things in other cells without having to tweak the axis positions. So yeah, nice fix 🎉

I could imagine splom always setting the defaults to 'bottom' and 'left'

Yeah, that's how it was initially, but c947b2c#diff-f4f66cea15079ea63f9be7a90528570c added this block for perf benefits.

alexcjohnson · 2018-10-04T18:19:53Z

Nice perseverance, excellent additions. Lets do it! 💃

etpinard added 11 commits September 26, 2018 13:28

update scattergl & splom 'text' attr description

56f2ee9

- scattergl 'text' now has the same behavior a scatter - splom 'text' is hover-only

move splom scene ref to fullLayout

3574ece

... and out of the gd.calcdata[i][0].t stash, so that: - scene refs can be relinked properly on updates - scene refs can be destroyed properly when needed - move also visibileDims out of calcdata stash to fullData[i], for convenience.

use styleOnSelect for scatter[polar]gl and splom

8d0cac5

... instead of 'style', for consistency. Other _module.style in the traces/ get called during Plots.style, something we don't want to do in regl traces, to avoid double-drawing.

compute marker.size axis 'ppad' value once per splom traces,

8282d91

... as opposed to once per splom trace dimensions.

merge options before matrix.update call

73acd7e

- matrix.update() can be expansive (something we should look into), so try to call it the least number of times possible.

speed up 'axrange' edits

cc5f0ca

- by just calling 'Axes.doTicks' instead of full `doTicksRelayout` which includes drawing the title - this also fixes (and locks) a bug where large splom would draw twice (once in doTicksRelayout and once in the drawData subroutine).

no need to re-calc 'regl' traces in-and-out of arrayOk values

3eb91c0

try implementing a fast 'markerSize' editType

145cad3

- marker.size is hard to edit fast, as in general it can change the axis ranges

etpinard added status: reviewable labels Sep 28, 2018

etpinard added this to the v1.42.0 milestone Sep 28, 2018