Skip to content

Aggressive splom perf #3057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 30 commits into from
Oct 4, 2018
Merged

Aggressive splom perf #3057

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
56f2ee9
update scattergl & splom 'text' attr description
etpinard Sep 18, 2018
3574ece
move splom scene ref to fullLayout
etpinard Sep 27, 2018
8d0cac5
use styleOnSelect for scatter[polar]gl and splom
etpinard Sep 27, 2018
8282d91
compute marker.size axis 'ppad' value once per splom traces,
etpinard Sep 27, 2018
73acd7e
merge options before matrix.update call
etpinard Sep 27, 2018
cc5f0ca
speed up 'axrange' edits
etpinard Sep 27, 2018
17e30d2
aggressively try to speed 'axrange' edits for splom
etpinard Sep 27, 2018
4792f08
optimize lsInner for splom
etpinard Sep 27, 2018
f415e96
first-cut editType:'style' pathway for sploms
etpinard Sep 27, 2018
3eb91c0
no need to re-calc 'regl' traces in-and-out of arrayOk values
etpinard Sep 27, 2018
145cad3
try implementing a fast 'markerSize' editType
etpinard Sep 27, 2018
6de1ae4
add supplyDefaults bypass 'axrange' optimization to Plotly.update
etpinard Oct 1, 2018
aaf6312
apply no-need-for-<rect.bg> optimization to all cartesian subplots
etpinard Oct 1, 2018
478c669
speed up splom.clean
etpinard Oct 1, 2018
81eb48b
introduce redrawReglTraces subroutine
etpinard Oct 2, 2018
0243451
:hocho: obsolete sortBasePlotModules
etpinard Oct 2, 2018
dcacd78
use redrawReglTraces on drag
etpinard Oct 2, 2018
68dfbc1
use redrawReglTraces on polar drag
etpinard Oct 2, 2018
c02330c
fix #2797 - clear full canvas and use redrawReglTraces on selections
etpinard Oct 2, 2018
02263b9
use redrawReglTraces in edit subroutines
etpinard Oct 2, 2018
f179c2f
skip canvas/context size mismatch test on CI
etpinard Oct 2, 2018
cd8d8ab
Merge pull request #3067 from plotly/redraw-regl-traces-subroutine
etpinard Oct 2, 2018
0aad7ea
fixup (add missing @gl)
etpinard Oct 2, 2018
078c086
clear axis types when restyling splom show(upper|lower)half
etpinard Oct 2, 2018
00cae48
Revert "clear axis types when restyling splom show(upper|lower)half"
etpinard Oct 3, 2018
5878223
make trace `(x|y)axes` always have the same length dimensions`
etpinard Oct 4, 2018
c2ecb3a
:hocho: obsolete args
etpinard Oct 4, 2018
31d5606
fixup comments
etpinard Oct 4, 2018
ea38664
add noCI tag to misbehaving tests
etpinard Oct 4, 2018
4137433
place splom axes on bottom/left sides when just diagonal is missing
etpinard Oct 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/components/fx/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,10 @@ function _hover(gd, evt, subplot, noHoverEvent) {
if(fullLayout[subplotId]) {
pointData.subplot = fullLayout[subplotId]._subplot;
}
// add ref to splom scene
if(fullLayout._splomScenes && fullLayout._splomScenes[trace.uid]) {
pointData.scene = fullLayout._splomScenes[trace.uid];
}

closedataPreviousLength = hoverData.length;

Expand Down
4 changes: 3 additions & 1 deletion src/plots/cartesian/select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,9 @@ function determineSearchTraces(gd, xAxes, yAxes, subplot) {
// FIXME: make sure we don't have more than single axis for splom
trace._xaxes[xAxisIds[0]] && trace._yaxes[yAxisIds[0]]
) {
searchTraces.push(createSearchInfo(trace._module, cd, xAxes[0], yAxes[0]));
var info = createSearchInfo(trace._module, cd, xAxes[0], yAxes[0]);
info.scene = gd._fullLayout._splomScenes[trace.uid];
searchTraces.push(info);
} else {
if(xAxisIds.indexOf(trace.xaxis) === -1) continue;
if(yAxisIds.indexOf(trace.yaxis) === -1) continue;
Expand Down
65 changes: 29 additions & 36 deletions src/traces/splom/base_plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,18 @@ function drag(gd) {
for(var i = 0; i < cd.length; i++) {
var cd0 = cd[i][0];
var trace = cd0.trace;
var stash = cd0.t;
var scene = stash._scene;
var scene = fullLayout._splomScenes[trace.uid];

if(trace.type === 'splom' && scene && scene.matrix) {
dragOne(gd, trace, stash, scene);
dragOne(gd, trace, scene);
}
}
}

function dragOne(gd, trace, stash, scene) {
function dragOne(gd, trace, scene) {
var visibleLength = scene.matrixOptions.data.length;
var visibleDims = stash.visibleDims;
var ranges = new Array(visibleLength);
var visibleDims = trace._visibleDims;
var ranges = scene.viewOpts.ranges = new Array(visibleLength);

for(var k = 0; k < visibleDims.length; k++) {
var i = visibleDims[k];
Expand Down Expand Up @@ -168,46 +167,40 @@ function makeGridData(gd) {
return gridBatches;
}

function clean(newFullData, newFullLayout, oldFullData, oldFullLayout, oldCalcdata) {
var oldModules = oldFullLayout._modules || [];
var newModules = newFullLayout._modules || [];
function clean(newFullData, newFullLayout, oldFullData, oldFullLayout) {
oldLoop:
for(var i = 0; i < oldFullData.length; i++) {
var oldTrace = oldFullData[i];

var hadSplom, hasSplom;
var i;
if(oldTrace.type === 'splom') {
for(var j = 0; j < newFullData.length; j++) {
var newTrace = newFullData[j];

for(i = 0; i < oldModules.length; i++) {
if(oldModules[i].name === 'splom') {
hadSplom = true;
break;
}
}
for(i = 0; i < newModules.length; i++) {
if(newModules[i].name === 'splom') {
hasSplom = true;
break;
}
}
if(oldTrace.uid === newTrace.uid && newTrace.type === 'splom') {
continue oldLoop;
}
}

if(hadSplom && !hasSplom) {
for(i = 0; i < oldCalcdata.length; i++) {
var cd0 = oldCalcdata[i][0];
var trace = cd0.trace;
var scene = cd0.t._scene;

if(
trace.type === 'splom' &&
scene && scene.matrix && scene.matrix.destroy
) {
scene.matrix.destroy();
cd0.t._scene = null;
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting... how did you figure this out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

oldFullLayout._splomScenes[oldTrace.uid] = null;
delete oldFullLayout._splomScenes[oldTrace.uid];
}
}
}

if(Object.keys(oldFullLayout._splomScenes || {}).length === 0) {
delete oldFullLayout._splomScenes;
}

if(oldFullLayout._splomGrid &&
(!newFullLayout._hasOnlyLargeSploms && oldFullLayout._hasOnlyLargeSploms)) {
// must first set scene to null in order to get garbage collected
oldFullLayout._splomGrid.destroy();
oldFullLayout._splomGrid = null;
delete oldFullLayout._splomGrid;
}

Cartesian.clean(newFullData, newFullLayout, oldFullData, oldFullLayout);
Expand All @@ -228,7 +221,7 @@ function updateFx(gd) {
var trace = cd0.trace;

if(trace.type === 'splom') {
var scene = cd0.t._scene;
var scene = fullLayout._splomScenes[trace.uid];
if(scene.selectBatch === null) {
scene.matrix.update(scene.matrixOptions, null);
}
Expand Down
59 changes: 32 additions & 27 deletions src/traces/splom/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,13 @@ var TOO_MANY_POINTS = require('../scattergl/constants').TOO_MANY_POINTS;
function calc(gd, trace) {
var dimensions = trace.dimensions;
var commonLength = trace._length;
var stash = {};
var opts = {};
// 'c' for calculated, 'l' for linear,
// only differ here for log axes, pass ldata to createMatrix as 'data'
var cdata = opts.cdata = [];
var ldata = opts.data = [];
// keep track of visible dimensions
var visibleDims = stash.visibleDims = [];
var visibleDims = trace._visibleDims = [];
var i, k, dim, xa, ya;

function makeCalcdata(ax, dim) {
Expand Down Expand Up @@ -107,22 +106,27 @@ function calc(gd, trace) {
calcAxisExpansion(gd, trace, xa, ya, cdata[k], cdata[k], ppad);
}

var scene = stash._scene = sceneUpdate(gd, stash);
var scene = sceneUpdate(gd, trace);
if(!scene.matrix) scene.matrix = true;
scene.matrixOptions = opts;

scene.selectedOptions = convertMarkerSelection(trace, trace.selected);
scene.unselectedOptions = convertMarkerSelection(trace, trace.unselected);

return [{x: false, y: false, t: stash, trace: trace}];
return [{x: false, y: false, t: {}, trace: trace}];
}

function sceneUpdate(gd, stash) {
var scene = stash._scene;
function sceneUpdate(gd, trace) {
var fullLayout = gd._fullLayout;
var uid = trace.uid;

var reset = {
dirty: true
};
// must place ref to 'scene' in fullLayout, so that:
// - it can be relinked properly on updates
// - it can be destroyed properly when needed
var splomScenes = fullLayout._splomScenes;
if(!splomScenes) splomScenes = fullLayout._splomScenes = {};

var reset = {dirty: true};

var first = {
selectBatch: null,
Expand All @@ -131,8 +135,10 @@ function sceneUpdate(gd, stash) {
select: null
};

var scene = splomScenes[trace.uid];

if(!scene) {
scene = stash._scene = Lib.extendFlat({}, reset, first);
scene = splomScenes[uid] = Lib.extendFlat({}, reset, first);

scene.draw = function draw() {
// draw traces in selection mode
Expand All @@ -147,13 +153,13 @@ function sceneUpdate(gd, stash) {

// remove scene resources
scene.destroy = function destroy() {
if(scene.matrix) scene.matrix.destroy();

if(scene.matrix && scene.matrix.destroy) {
scene.matrix.destroy();
}
scene.matrixOptions = null;
scene.selectBatch = null;
scene.unselectBatch = null;

stash._scene = null;
scene = null;
};
}

Expand All @@ -178,7 +184,7 @@ function plotOne(gd, cd0) {
var gs = fullLayout._size;
var trace = cd0.trace;
var stash = cd0.t;
var scene = stash._scene;
var scene = fullLayout._splomScenes[trace.uid];
var matrixOpts = scene.matrixOptions;
var cdata = matrixOpts.cdata;
var regl = fullLayout._glcanvas.data()[0].regl;
Expand All @@ -194,7 +200,7 @@ function plotOne(gd, cd0) {
matrixOpts.upper = trace.showlowerhalf;
matrixOpts.diagonal = trace.diagonal.visible;

var visibleDims = stash.visibleDims;
var visibleDims = trace._visibleDims;
var visibleLength = cdata.length;
var viewOpts = {};
viewOpts.ranges = new Array(visibleLength);
Expand Down Expand Up @@ -305,17 +311,16 @@ function plotOne(gd, cd0) {
function hoverPoints(pointData, xval, yval) {
var cd = pointData.cd;
var trace = cd[0].trace;
var stash = cd[0].t;
var scene = stash._scene;
var scene = pointData.scene;
var cdata = scene.matrixOptions.cdata;
var xa = pointData.xa;
var ya = pointData.ya;
var xpx = xa.c2p(xval);
var ypx = ya.c2p(yval);
var maxDistance = pointData.distance;

var xi = getDimIndex(trace, stash, xa);
var yi = getDimIndex(trace, stash, ya);
var xi = getDimIndex(trace, xa);
var yi = getDimIndex(trace, ya);
if(xi === false || yi === false) return [pointData];

var x = cdata[xi];
Expand Down Expand Up @@ -352,7 +357,7 @@ function selectPoints(searchInfo, selectionTester) {
var cd = searchInfo.cd;
var trace = cd[0].trace;
var stash = cd[0].t;
var scene = stash._scene;
var scene = searchInfo.scene;
var cdata = scene.matrixOptions.cdata;
var xa = searchInfo.xaxis;
var ya = searchInfo.yaxis;
Expand All @@ -364,8 +369,8 @@ function selectPoints(searchInfo, selectionTester) {
var hasOnlyLines = (!subTypes.hasMarkers(trace) && !subTypes.hasText(trace));
if(trace.visible !== true || hasOnlyLines) return selection;

var xi = getDimIndex(trace, stash, xa);
var yi = getDimIndex(trace, stash, ya);
var xi = getDimIndex(trace, xa);
var yi = getDimIndex(trace, ya);
if(xi === false || yi === false) return selection;

var xpx = stash.xpx[xi];
Expand Down Expand Up @@ -424,15 +429,15 @@ function style(gd, cds) {

var fullLayout = gd._fullLayout;
var cd0 = cds[0];
var scene0 = cd0[0].t._scene;
var scene0 = fullLayout._splomScenes[cd0[0].trace.uid];
scene0.matrix.regl.clear({color: true, depth: true});

if(fullLayout._splomGrid) {
fullLayout._splomGrid.draw();
}

for(var i = 0; i < cds.length; i++) {
var scene = cds[i][0].t._scene;
var scene = fullLayout._splomScenes[cds[i][0].trace.uid];
scene.draw();
}

Expand All @@ -446,11 +451,11 @@ function style(gd, cds) {
}
}

function getDimIndex(trace, stash, ax) {
function getDimIndex(trace, ax) {
var axId = ax._id;
var axLetter = axId.charAt(0);
var ind = {x: 0, y: 1}[axLetter];
var visibleDims = stash.visibleDims;
var visibleDims = trace._visibleDims;

for(var k = 0; k < visibleDims.length; k++) {
var i = visibleDims[k];
Expand Down
41 changes: 27 additions & 14 deletions test/jasmine/tests/splom_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,11 @@ describe('Test splom trace calc step:', function() {
yaxis: {type: 'linear'}
});

var cd = gd.calcdata[0][0];
var trace = gd._fullData[0];
var scene = gd._fullLayout._splomScenes[trace.uid];

expect(cd.t._scene.matrixOptions.data).toBeCloseTo2DArray([[2, 1, 2]]);
expect(cd.t.visibleDims).toEqual([1]);
expect(scene.matrixOptions.data).toBeCloseTo2DArray([[2, 1, 2]]);
expect(trace._visibleDims).toEqual([1]);
expect(Lib.log).toHaveBeenCalledTimes(1);
expect(Lib.log).toHaveBeenCalledWith('Skipping splom dimension 0 with conflicting axis types');
});
Expand All @@ -448,13 +449,14 @@ describe('Test splom interactions:', function() {

Plotly.plot(gd, fig).then(function() {
expect(gd._fullLayout._splomGrid).toBeDefined();
expect(gd.calcdata[0][0].t._scene).toBeDefined();
expect(gd._fullLayout._splomScenes).toBeDefined();
expect(Object.keys(gd._fullLayout._splomScenes).length).toBe(1);

return Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout, gd.calcdata);
return Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout);
})
.then(function() {
expect(gd._fullLayout._splomGrid).toBe(null);
expect(gd.calcdata[0][0].t._scene).toBe(null);
expect(gd._fullLayout._splomGrid).toBeUndefined();
expect(gd._fullLayout._splomScenes).toBeUndefined();
})
.catch(failTest)
.then(done);
Expand Down Expand Up @@ -658,14 +660,22 @@ describe('Test splom interactions:', function() {
var fig = Lib.extendDeep({}, require('@mocks/splom_iris.json'));

function _assert(msg, exp) {
var splomScenes = gd._fullLayout._splomScenes;
var ids = Object.keys(splomScenes);

for(var i = 0; i < 3; i++) {
expect(Boolean(gd.calcdata[i][0].t._scene))
.toBe(Boolean(exp[i]), msg + ' - trace ' + i);
var drawFn = splomScenes[ids[i]].draw;
expect(drawFn).toHaveBeenCalledTimes(exp[i], msg + ' - trace ' + i);
drawFn.calls.reset();
}
}

Plotly.plot(gd, fig).then(function() {
_assert('base', [1, 1, 1]);
var splomScenes = gd._fullLayout._splomScenes;
for(var k in splomScenes) {
spyOn(splomScenes[k], 'draw').and.callThrough();
}

return Plotly.restyle(gd, 'visible', 'legendonly', [0, 2]);
})
.then(function() {
Expand Down Expand Up @@ -890,7 +900,8 @@ describe('Test splom drag:', function() {

Plotly.plot(gd, fig)
.then(function() {
var scene = gd.calcdata[0][0].t._scene;
var uid = gd._fullData[0].uid;
var scene = gd._fullLayout._splomScenes[uid];
spyOn(scene.matrix, 'update');
spyOn(scene.matrix, 'draw');

Expand All @@ -906,7 +917,8 @@ describe('Test splom drag:', function() {
})
.then(function() { return _drag([130, 130], [150, 150]); })
.then(function() {
var scene = gd.calcdata[0][0].t._scene;
var uid = gd._fullData[0].uid;
var scene = gd._fullLayout._splomScenes[uid];
// N.B. _drag triggers two updateSubplots call
// - 1 update and 1 draw call per updateSubplot
// - 2 update calls (1 for data, 1 for view opts)
Expand Down Expand Up @@ -1107,7 +1119,7 @@ describe('Test splom select:', function() {
grid: {xgap: 0, ygap: 0}
};

var scene;
var uid, scene;

function _assert(msg, exp) {
expect(scene.matrix.update).toHaveBeenCalledTimes(exp.updateCnt, 'update cnt');
Expand All @@ -1122,7 +1134,8 @@ describe('Test splom select:', function() {
}

Plotly.plot(gd, fig).then(function() {
scene = gd.calcdata[0][0].t._scene;
uid = gd._fullData[0].uid;
scene = gd._fullLayout._splomScenes[uid];
spyOn(scene.matrix, 'update').and.callThrough();
spyOn(scene.matrix, 'draw').and.callThrough();
})
Expand Down