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

Speed up cartesian axes supplyDefaults #2549

Closed
etpinard opened this issue Apr 11, 2018 · 6 comments
Closed

Speed up cartesian axes supplyDefaults #2549

etpinard opened this issue Apr 11, 2018 · 6 comments

Comments

@etpinard
Copy link
Contributor

From #2527, Plots.supplyDefaults takes > 100ms at 50x50 subplots which feels like an eternity when zooming.

  • Getting rid of indexOf into axis lists would be a good start (perhaps experimenting with ES6 Map and Set could be worthwhile)
  • Perhaps we could try opening a partial Plots.supplyDefaults pathway, that would only update a subset of the graph's cartesian axes and their dependencies.
This was referenced Apr 11, 2018
@etpinard
Copy link
Contributor Author

etpinard commented Apr 19, 2018

Some more observations:

  • at 50x50 subplots, about 95% of the Plots.supplyDefaults duration takes place in supplyLayoutModuleDefaults (which coerces all those axes) and Plots.linkSubplots. The latter is especially heavy for multi-subplot graphs. Here:

for(i = 0; i < axList.length; i++) {
// Figure out which subplot to draw ticks, labels, & axis lines on
// do this as a separate loop so we already have all the
// _mainAxis and _anchorAxis links set
ax = axList[i];
var isX = ax._id.charAt(0) === 'x';
var anchorAx = ax._mainAxis._anchorAxis;
var mainSubplotID = '';
var nextBestMainSubplotID = '';
var anchorID = '';
// First try the main ID with the anchor
if(anchorAx) {
anchorID = anchorAx._mainAxis._id;
mainSubplotID = isX ? (ax._id + anchorID) : (anchorID + ax._id);
}
// Then look for a subplot with the counteraxis overlaying the anchor
// If that fails just use the first subplot including this axis
if(!mainSubplotID || ids.indexOf(mainSubplotID) === -1) {
mainSubplotID = '';
for(j = 0; j < ids.length; j++) {
id = ids[j];
var yIndex = id.indexOf('y');
var idPart = isX ? id.substr(0, yIndex) : id.substr(yIndex);
var counterPart = isX ? id.substr(yIndex) : id.substr(0, yIndex);
if(idPart === ax._id) {
if(!nextBestMainSubplotID) nextBestMainSubplotID = id;
var counterAx = axisIDs.getFromId(mockGd, counterPart);
if(anchorID && counterAx.overlaying === anchorID) {
mainSubplotID = id;
break;
}
}
}
}
ax._mainSubplot = mainSubplotID || nextBestMainSubplotID;
}

for every anchor-less axis, it loops over all subplots.

So I'm thinking of moving this block out of Plots.linkSubplots and into the lsInner subroutine so that "easy" edit types such as 'legend', 'axrange', 'ticks' and 'modebar' don't have to go through it. Referencing #2227 (comment) where something similar was proposed.

@alexcjohnson
Copy link
Collaborator

So I'm thinking of moving this block out of Plots.linkSubplots and into the lsInner subroutine

Sounds reasonable. Another option - not necessarily mutually exclusive, we could do both in fact - would be to make another collation of subplots that maps each (x|y) axis onto the (y|x) axes it makes subplots with. That should substantially reduce the size and complexity of loops like this.

@etpinard
Copy link
Contributor Author

So I'm thinking of moving this block out of Plots.linkSubplots and into the lsInner subroutine

Done in #2628.

PR #2628 also added set axis anchors to all sploms except the ones with showlowerhalf: false so that the "find main subplot" loop is skipped when we can.

@etpinard
Copy link
Contributor Author

etpinard commented Oct 3, 2018

From #3070 (comment), 50 dims splom traces spend about 40ms in axis_autotype.js. Improving perf there could be very beneficial.

@etpinard
Copy link
Contributor Author

Referencing #749 - moving away from the "relink everything approach" should speed up supplyDefaults in some scenarios.

@gvwilson
Copy link
Contributor

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson

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

No branches or pull requests

3 participants