Fix relayout constrained axes + various multi-subplot perf improvements #2628

etpinard · 2018-05-11T18:18:03Z

This PR started with a fix for relayout on constrained axes #2540 (comment). This fix made the rangesAltered stash in relayout

plotly.js/src/plot_api/plot_api.js

Lines 2028 to 2047 in a132b85

    
           // figure out if we need to recalculate axis constraints 
        
           var constraints = fullLayout._axisConstraintGroups || []; 
        
           for(axId in rangesAltered) { 
        
               for(i = 0; i < constraints.length; i++) { 
        
                   var group = constraints[i]; 
        
                   if(group[axId]) { 
        
                       // Always recalc if we're changing constrained ranges. 
        
                       // Otherwise it's possible to violate the constraints by 
        
                       // specifying arbitrary ranges for all axes in the group. 
        
                       // this way some ranges may expand beyond what's specified, 
        
                       // as they do at first draw, to satisfy the constraints. 
        
                       flags.calc = true; 
        
                       for(var groupAxId in group) { 
        
                           if(!rangesAltered[groupAxId]) { 
        
                               Axes.getFromId(gd, groupAxId)._constraintShrinkable = true; 
        
                           } 
        
                       } 
        
                   } 
        
               } 
        
           }

work properly and lead me to use the same stash to implement per-axis "axrange" edits (as planned in #2547). While at it, I added a few more splom-related perf improvements for multi-subplot graphs, namely

skip Axes.makeClipPaths under hasOnlyLargeSploms planned in Unnecessary clipPaths #2595 (comment)
move costly "find main subplot" for all anchor-less axes loop out of supplyDefaults -> lsInner as planned in Speed up cartesian axes supplyDefaults #2549 (comment)
made splom generated axes set an anchor axis so that that costly "find main subplot" loop is skipped except when the lower half is omitted.

Some benchmarks:

Implementing per-axis "axrange" relayout edits trims ~200ms on e.g Plotly.relayout(gd, 'xaxis.range[0], 0) on a 50-dimensions splom. At 15 dimensions (i.e. with svg grid lines), it trims ~100ms.
Making all sploms except those with showlowerhalf: false have a set anchor trims 40-50ms.

So, starting with https://codepen.io/etpinard/pen/wjmqmO and benchmarking Plotly.relayout(gd, 'xaxis.range', [0, 1]) we get:

# dims	# values per dim	results w/ 1.37.1 [ms]	results w/ this branch [ms]
15	100	160	40
30	100	220	130
50	100	500	250

... by extracting correct prop "leaf" end index which is used to keep track of "altered ranges" which than lead to `flags.calc: true` under axis constraints. In the test added, the # of xgrid ended being the same before and after the relayout call.

- updating axes with constraints triggers a 'calc' edit, so no need to do this here.

- these clip paths are only used for shapes and layout images, which make hasOnlyLargeSploms false, so we don't need them. - this can trim ~50ms on first render at 50 splom dimensions.

- by passing (now correct) alteredRange hash object to doTicksRelayout - by addding new doTicks(gd, [axId, axId2, ...]) call signature - clean up doTicks docstring - N.B. not implemented for Plotly.react (yet)

- so that supplyDefaults and "easy" edits (e.g. axrange, modebar) can be faster. - mv "axrange" react test out of relayout switchboard describe keeping "real" graph div out the relayout switchboard describe

- this way, the Grid component can generate axes with set 'anchor' for all sploms except when `showlowerhalf: false`. Having set anchored axes is a performance boost (~40ms at 50 dimensions).

... which had their y-axis shift a little bit by the previous commit that made their axes anchored.

... which in my opinion is now more "correct" than previously

etpinard · 2018-05-11T18:23:28Z

A few more comments on constrained axes, that should be followed up in #2540:

Could making range relayout go through editType "plot" (instead of "calc") be sufficient? When replacing this line

plotly.js/src/plot_api/plot_api.js

Line 2039 in a132b85

flags.calc = true;

with flags.plot = true, no tests are failing.

I think Plotly.react is broken with constrained axes. I'm not sure if it's always been broken or if the recently-added "axrange" edit did it 😕

etpinard · 2018-05-11T18:27:11Z

src/plot_api/plot_api.js

@@ -2171,7 +2169,9 @@ exports.update = function update(gd, traceUpdate, layoutUpdate, _traces) {
        if(relayoutFlags.layoutstyle) seq.push(subroutines.layoutStyles);
        if(relayoutFlags.axrange) {
            seq.push(
-                subroutines.doTicksRelayout,
+                function(gd) {
+                    return subroutines.doTicksRelayout(gd, relayoutSpecs.rangesAltered);


N.B. @alexcjohnson I didn't implement per-axis "axrange" edit for Plotly.react. My main concern was improvement perf on dragend axis range relayout calls. I didn't think adding an exception to the react diffing algo was worth it for this case.

etpinard · 2018-05-11T18:29:52Z

src/plot_api/subroutines.js

@@ -324,6 +329,48 @@ exports.lsInner = function(gd) {
    return gd._promises.length && Promise.all(gd._promises);
 };

+function findMainSubplot(ax, fullLayout) {


@alexcjohnson I didn't bother making the "find main subplot" algo faster as you proposed in #2549 (comment), I was happy enough with skipping this whole thing except for sploms with showlowerhalf: false with c947b2c

alexcjohnson · 2018-05-11T19:11:48Z

Could making range relayout go through editType "plot" (instead of "calc") be sufficient?

Do we have a test of setting mutually-exclusive range values, as per the comment on that line? If we do, and the result correctly fixes them for the constraints, then go for it!

alexcjohnson · 2018-05-11T19:19:45Z

src/plot_api/plot_api.js

@@ -1806,7 +1806,7 @@ function _relayout(gd, aobj) {
        var plen = p.parts.length;
        // p.parts may end with an index integer if the property is an array
        var pend = plen - 1;
-        while(pend > 0 && typeof p.parts[plen - 1] !== 'string') { pend--; }
+        while(pend > 0 && typeof p.parts[pend] !== 'string') pend--;


Ha, nice catch!

alexcjohnson · 2018-05-11T19:41:00Z

src/plots/cartesian/axes.js

+ *
+ * Signature 5: Axes.doTicks(gd, [axId, axId2, ...])
+ *   where the items are axis id string,
+ *   use this to update multiple axes in one call


This was already a bit out of control, do we really want to add another signature?

Can we perhaps break it into 2 separate functions, one for signatures 1 & 2 and a second for 3, 4, 5? That's how it functions anyway - 3,4,5 just map into a bunch of calls to signature 2 (ignoring skipTitle) and then return.

Sure that would be nice. But since doTicks is one of our most referenced function in existing issues, I'm a little reluctant to do so.

But if you insist, than I'm thinking of making Axes.doTicks the multi-cartesian-axis version (that accepts '', 'redraw' and arrays of axis ids), and adding Axes.doTicksSingle (that accepts one axis id string or an axis object).

On master, we have:

alexcjohnson · 2018-05-11T21:39:44Z

💪 Well done, this is a really fantastic collection of boosts and fixes! Thanks for the Axes.doTicks separation, a nice first step toward more comprehensible axes.js.

#2628 (comment) is non-blocking.
💃

etpinard · 2018-05-14T14:10:24Z

#2628 (comment) is non-blocking.

Moved to #2540 (comment)

etpinard added 10 commits May 10, 2018 18:05

rm doAutoRangeAndConstraints step from "axrange" edits

23964ca

- updating axes with constraints triggers a 'calc' edit, so no need to do this here.

move axis range/autorange/domain regex to module scope

abd4bcf

skip Axes.makeClipPaths under hasOnlyLargeSploms regime

09d5dd0

- these clip paths are only used for shapes and layout images, which make hasOnlyLargeSploms false, so we don't need them. - this can trim ~50ms on first render at 50 splom dimensions.

resolves #2547 - target altered axes only on "axrange" edits

5557d7b

- by passing (now correct) alteredRange hash object to doTicksRelayout - by addding new doTicks(gd, [axId, axId2, ...]) call signature - clean up doTicks docstring - N.B. not implemented for Plotly.react (yet)

only grab 2d axes in lsInner

73b354b

mv find-main-subplot logic from supplyDefaults -> lsInner

e69b7dd

- so that supplyDefaults and "easy" edits (e.g. axrange, modebar) can be faster. - mv "axrange" react test out of relayout switchboard describe keeping "real" graph div out the relayout switchboard describe

only override grid.(x|y)side default for sploms w/o lower half

c947b2c

- this way, the Grid component can generate axes with set 'anchor' for all sploms except when `showlowerhalf: false`. Having set anchored axes is a performance boost (~40ms at 50 dimensions).

update splom baselines

6fa8ffd

... which had their y-axis shift a little bit by the previous commit that made their axes anchored.

update splom_ragged-via-axes baseline

455ff4d

... which in my opinion is now more "correct" than previously

etpinard added bug something broken status: reviewable labels May 11, 2018

etpinard commented May 11, 2018

View reviewed changes

alexcjohnson reviewed May 11, 2018

View reviewed changes

split doTicks into multi-axes and doTicksSingle parts

72b6558

etpinard mentioned this pull request May 14, 2018

Investigate relayout & react + axis constraints behavior #2540

Closed

etpinard merged commit 6fbdf23 into master May 14, 2018

etpinard deleted the fixup-rerange-axis-constraints branch May 14, 2018 14:14

This was referenced May 14, 2018

Unnecessary clipPaths #2595

Closed

Speed up cartesian axes supplyDefaults #2549

Closed

Per axis axrange edit type #2547

Closed

etpinard mentioned this pull request Jul 18, 2018

Faster axis autorange + remove calcIfAutorange edit type #2823

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix relayout constrained axes + various multi-subplot perf improvements #2628

Fix relayout constrained axes + various multi-subplot perf improvements #2628

etpinard commented May 11, 2018 •

edited

Loading

etpinard commented May 11, 2018 •

edited

Loading

etpinard May 11, 2018 •

edited

Loading

etpinard May 11, 2018 •

edited

Loading

alexcjohnson commented May 11, 2018

alexcjohnson May 11, 2018

alexcjohnson May 11, 2018

etpinard May 11, 2018 •

edited

Loading

alexcjohnson commented May 11, 2018

etpinard commented May 14, 2018

	// figure out if we need to recalculate axis constraints
	var constraints = fullLayout._axisConstraintGroups \|\| [];
	for(axId in rangesAltered) {
	for(i = 0; i < constraints.length; i++) {
	var group = constraints[i];
	if(group[axId]) {
	// Always recalc if we're changing constrained ranges.
	// Otherwise it's possible to violate the constraints by
	// specifying arbitrary ranges for all axes in the group.
	// this way some ranges may expand beyond what's specified,
	// as they do at first draw, to satisfy the constraints.
	flags.calc = true;
	for(var groupAxId in group) {
	if(!rangesAltered[groupAxId]) {
	Axes.getFromId(gd, groupAxId)._constraintShrinkable = true;
	}
	}
	}
	}
	}

Fix relayout constrained axes + various multi-subplot perf improvements #2628

Fix relayout constrained axes + various multi-subplot perf improvements #2628

Conversation

etpinard commented May 11, 2018 • edited Loading

etpinard commented May 11, 2018 • edited Loading

etpinard May 11, 2018 • edited Loading

Choose a reason for hiding this comment

etpinard May 11, 2018 • edited Loading

Choose a reason for hiding this comment

alexcjohnson commented May 11, 2018

alexcjohnson May 11, 2018

Choose a reason for hiding this comment

alexcjohnson May 11, 2018

Choose a reason for hiding this comment

etpinard May 11, 2018 • edited Loading

Choose a reason for hiding this comment

alexcjohnson commented May 11, 2018

etpinard commented May 14, 2018

etpinard commented May 11, 2018 •

edited

Loading

etpinard commented May 11, 2018 •

edited

Loading

etpinard May 11, 2018 •

edited

Loading

etpinard May 11, 2018 •

edited

Loading

etpinard May 11, 2018 •

edited

Loading