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

Faster axis autorange relayout #2546

Closed
etpinard opened this issue Apr 11, 2018 · 2 comments · Fixed by #2823
Closed

Faster axis autorange relayout #2546

etpinard opened this issue Apr 11, 2018 · 2 comments · Fixed by #2823

Comments

@etpinard
Copy link
Contributor

A follow-up on #2527 where I first noticed:

double-click (i.e auto/reset range) performance. This thing currently goes through editType: 'calc' which is slow as a turtle. I'll try to cache the autorange value somewhere (and clearing it when new data comes in) and use the axrange pathway to speed thing up. This will benefit big data gl and svg graphs too!

and @alexcjohnson replied:

Axes.expand (which happens during calc does nothing if the axis isn't autoranged - which can speed up initial drawing but means if you turn on autorange later you don't know whether ax._min/_max are populated. We could either calculate min/max no matter what during calc, or at least stash the arrays & options used to generate them so that when autorange is enabled we can pull it back out and proceed with the calculation without needing to go all the way through calc again.

There would also be some advantages to keeping separate track of each trace / object that called Axes.expand - ie keep a separate _min/_max for each trace/shape/annotation etc. and only combine them when finally calculating the range. That would avoid calc for visibility changes, for example (you'd just have to know to include or not include that object's _min/_max) and for moving annotations/shapes, and down the road would allow calc per trace.


Getting this right will improve double-click axis autorange performance and play an important role in #2358 (faster extendTraces) and other autorange-related improvements (e.g. #387)

@etpinard
Copy link
Contributor Author

etpinard commented Jul 16, 2018

Swapping editType: 'calc' for editType: 'plot' in the axis autorange declaration went almost too smoothly. Just one test was falling (apart from tests checking for undefined ax._min and ax._max) :

it('when set to \'reset+autorange\' (the default) should follow updated auto ranges', function(done) {
var updateData = {
x: [[1e-4, 0, 1e3]],
y: [[30, 0, 30]]
};
var newAutoRangeX = [-4.482371794871794, 3.4823717948717943],
newAutoRangeY = [-0.8892256657741471, 1.6689872212461876];
Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() {
expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX);
expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY);
return Plotly.relayout(gd, update);
}).then(function() {
expect(gd.layout.xaxis.range).toBeCloseToArray(zoomRangeX);
expect(gd.layout.yaxis.range).toBeCloseToArray(zoomRangeY);
return Plotly.restyle(gd, updateData);
}).then(function() {
expect(gd.layout.xaxis.range).toBeCloseToArray(zoomRangeX);
expect(gd.layout.yaxis.range).toBeCloseToArray(zoomRangeY);
return doubleClick(blankPos[0], blankPos[1]);
}).then(function() {
expect(gd.layout.xaxis.range).toBeCloseToArray(newAutoRangeX);
expect(gd.layout.yaxis.range).toBeCloseToArray(newAutoRangeY);
return doubleClick(blankPos[0], blankPos[1]);
}).then(function() {
expect(gd.layout.xaxis.range).toBeCloseToArray(newAutoRangeX);
expect(gd.layout.yaxis.range).toBeCloseToArray(newAutoRangeY);
done();
});
});

By this old little test was enough to highlight a pretty important problem with this approach and 'calcIfAutorange' edits - which has the name suggests leads to a calc edit pathway if one or more axis has autorange: true.

For example, scatter marker.size is a 'calcIfAutorange' attribute as it can expand the axis ranges. So consider,

Plotly.newPlot(gd, [{
  y: [1, 2, 1] 
}], {
  xaxis: {range: [0, 2]},
  yaxis: {range: [0, 2]}
})

// then
Plotly.restyle(gd, 'marker.size', 30)

// then
Plotly.relayout(gd, {
  'xaxis.autorange': true,
  'yaxis.autorange': true
})

// then 
gd.layout.xaxis.range
// (on master) => [-0.18848653667595172, 2.188486536675952]
// (with editType: 'plot' autorange) => [0, 2]
//  i.e.  the autorange change doesn't get picked up!

There are a few ways to solve the problem:

  1. 🔪 the 'calcIfAutorange' edit type and replace it with 'calc'. This solution is easy to implement, but leads to slower update pathways.
  2. replace 'calcIfAutorange' with something like 'axexpand+plot', that only updates ax._min / ax._max accordingly as opposed to going through the entire calc pathway.
  3. something to 2) but with per-trace update as proposed in the issue description above

Personally, I think speeding up double-click and legend visible trace toggling is more important than keeping marker.size edits and the few other 'calcIfAutorange' attributes (mostly in annotations and shapes) updating fast when the range is set. If some of you disagree, please let me know.

@etpinard
Copy link
Contributor Author

Oh and here are some benchmarks for Axes.expand (with autorange: true):

Scatter at 1e4 pts:

array marker.size time [ms] % of total render time
yes 20 < 1%
no 2-5 < 1%

Scattergl at 1e5-1 pts (the worse case before the "mean(marker.size)" approximation):

array marker.size time [ms] % of total render time
yes 60 - 80 ~10%
no 5 < 1%

Scattergl at 1e6 pts:

array marker.size time [ms] % of total render time
yes ~10 < 1%
no ~10 < 1%

All in all, I think skipping Axes.expand during calc when axis ranges are set is an unnecessary optimization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant