Skip to content
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.

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

Aggressive splom perf #3057

Merged
merged 30 commits into from
Oct 4, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
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
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: 2 additions & 2 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -1738,8 +1738,8 @@ function addAxRangeSequence(seq, rangesAltered) {
// subroutine of its own so that finalDraw always gets
// executed after drawData
var doTicks = rangesAltered ?
function(gd) { return subroutines.doTicksRelayout(gd, rangesAltered); } :
subroutines.doTicksRelayout;
function(gd) { return Axes.doTicks(gd, Object.keys(rangesAltered), true); } :
function(gd) { return Axes.doTicks(gd, 'redraw'); };
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved

seq.push(
subroutines.doAutoRangeAndConstraints,
Expand Down
8 changes: 2 additions & 6 deletions src/plot_api/subroutines.js
Original file line number Diff line number Diff line change
Expand Up @@ -498,12 +498,8 @@ exports.doLegend = function(gd) {
return Plots.previousPromises(gd);
};

exports.doTicksRelayout = function(gd, rangesAltered) {
if(rangesAltered) {
Axes.doTicks(gd, Object.keys(rangesAltered), true);
} else {
Axes.doTicks(gd, 'redraw');
}
exports.doTicksRelayout = function(gd) {
Axes.doTicks(gd, 'redraw');

if(gd._fullLayout._hasOnlyLargeSploms) {
clearGlCanvases(gd);
Expand Down
4 changes: 3 additions & 1 deletion src/plots/cartesian/select.js
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
10 changes: 1 addition & 9 deletions src/traces/scattergl/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,7 @@ var attrs = module.exports = overrideAll({
y0: scatterAttrs.y0,
dy: scatterAttrs.dy,

text: extendFlat({}, scatterAttrs.text, {
description: [
'Sets text elements associated with each (x,y) pair to appear on hover.',
'If a single string, the same string appears over',
'all the data points.',
'If an array of string, the items are mapped in order to the',
'this trace\'s (x,y) coordinates.'
].join(' ')
}),
text: scatterAttrs.text,
hovertext: scatterAttrs.hovertext,

textposition: scatterAttrs.textposition,
Expand Down
6 changes: 2 additions & 4 deletions src/traces/scattergl/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -893,9 +893,7 @@ function selectPoints(searchInfo, selectionTester) {
return selection;
}

function style(gd, cds) {
if(!cds) return;

function styleOnSelect(gd, cds) {
var stash = cds[0][0].t;
var scene = stash._scene;

Expand Down Expand Up @@ -957,7 +955,7 @@ module.exports = {
calc: calc,
plot: plot,
hoverPoints: hoverPoints,
style: style,
styleOnSelect: styleOnSelect,
selectPoints: selectPoints,

sceneOptions: sceneOptions,
Expand Down
2 changes: 1 addition & 1 deletion src/traces/scatterpolargl/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ module.exports = {
calc: calc,
plot: plot,
hoverPoints: hoverPoints,
style: ScatterGl.style,
styleOnSelect: ScatterGl.styleOnSelect,
selectPoints: ScatterGl.selectPoints,

meta: {
Expand Down
11 changes: 10 additions & 1 deletion src/traces/splom/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
var scatterGlAttrs = require('../scattergl/attributes');
var cartesianIdRegex = require('../../plots/cartesian/constants').idRegex;
var templatedArray = require('../../plot_api/plot_template').templatedArray;
var extendFlat = require('../../lib/extend').extendFlat;

function makeAxesValObject(axLetter) {
return {
Expand Down Expand Up @@ -86,7 +87,15 @@ module.exports = {

// mode: {}, (only 'markers' for now)

text: scatterGlAttrs.text,
text: extendFlat({}, scatterGlAttrs.text, {
description: [
'Sets text elements associated with each (x,y) pair to appear on hover.',
'If a single string, the same string appears over',
'all the data points.',
'If an array of string, the items are mapped in order to the',
'this trace\'s (x,y) coordinates.'
].join(' ')
}),
marker: scatterGlAttrs.marker,

xaxes: makeAxesValObject('x'),
Expand Down
65 changes: 29 additions & 36 deletions src/traces/splom/base_plot.js
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;
}
}
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved

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
Loading