Merge diets

[rleir]

Description of proposed changes

What is the goal of this pull request? What does this pull request change?
Performance in SVG updates.
Note: this PR is experimental, not ready for prime time. And I probably have not found the best way to measure performance here. Note also that there is a visual difference, with the tip borders all the same colour (black) which might be a bit visually jarring. But when we have the tip count increasing weekly we need to make plans.

This branch merges #1128 SvgDiet, #1136 PrecisionDiet and #1137 PrecisionDiet2 for some measurements. These changes all reduce the DOM storage for tips and branches. During SVG changes it would be nice if all this storage could fit in processor cache, but of course it cannot.

Related issue(s)

Fixes #
Related to # performance audit

Testing

What steps should be taken to test the changes you've proposed?
If you added or changed behavior in the codebase, did you update the tests, or do you need help with this?
I added two timers in components/tree/phyloTree/change.js named
"4.modifySVG"
"4phylotree.change"

Here is the code being timed:

  timerStart(" 4.modifySVG"); // line154
  classesToPotentiallyUpdate.forEach((el) => {
    if (elemsToUpdate.has(el)) {
      updateCall = createUpdateCall(el, svgPropsToUpdate);
      genericSelectAndModify(this.svg, el, updateCall, transitionTime);
    }
  });
  timerEnd(" 4.modifySVG"); //line 161

 timerStart("4phylotree.change()"); // line 373
 /* Finally, actually change the SVG elements themselves */
 const extras = { removeConfidences, showConfidences, newBranchLabellingKey };
 extras.timeSliceHasPotentiallyChanged = changeVisibility || newDistance;
 extras.hideTipLabels = animationInProgress;
 if (useModifySVGInStages) {
   this.modifySVGInStages(elemsToUpdate, svgPropsToUpdate, transitionTime, 1000);
 } else {
   this.modifySVG(elemsToUpdate, svgPropsToUpdate, transitionTime, extras);
 }
 this.timeLastRenderRequested = Date.now();
 timerEnd("4phylotree.change()"); // line 384

The dataset: http://localhost:4000/ncov/north-america dates to may8
The timings below are a bit irregular, but at first glance there is an improvement over master. (The build still has other timers such as addgrid, entropy which I am not currently interested in, so they are not shown).

branch: master
click to change the date slider slightly

  4.modifySVG     took 1220ms
                        took 1219ms

click a short vertical line at bottom-left with "descendents: 5305"
to zoom in on a clade

 4phylotree.change() took 972ms. 
 4.modifySVG          took 747ms.  

reset layout

 4.modifySVG          took 763ms
 4phylotree.change() took 950ms. 

branch:mergeDiets
click to change the date slider slightly

  4.modifySVG (#2)          took 130ms.
  4.modifySVG (#3)          took 519ms.
  4.modifySVG (#11)         took 546ms.

click a short vertical line at bottom-left with "descendents: 5305"
to zoom in on a clade

  4phylotree.change() (#13) took 274ms.

click to return out from clade

  4phylotree.change() (#24) took 244ms.

reset layout

click to zoom in on a clade (choose a large one

  4phylotree.change() (#33) took 295ms.

reset layout

 4.modifySVG (#35) took 18ms.
 4phylotree.change() (#35) took 138ms.

Thank you for contributing to Nextstrain!

[rleir]

Now I will be a bit more careful to note the details of the test.
We are comparing the mergedDiets branch to master. There are 6 timerStart/End's commented out just to stop some timers from cluttering the console in the timeframes of the tests.
The dataset is as before:
dataset: http://localhost:4000/ncov/north-america dates to may8
Method: in Chrome, open the inspect->console and note the timer measurements. The console scrolls and to keep track of your recent position you can select a word of text just to highlight it. This is all a bit laborious, I need to automate it.

Equipment: a 5-year old i5
Server: localhost running in develop mode
initial state: "Reset Layout" then the site was left untouched for a few minutes. Avoid hovering over the tree, as this causes clutter in the console.
The timer was added in phyloTree/change.js around lines 154 to 160.

  classesToPotentiallyUpdate.forEach((el) => {
    if (elemsToUpdate.has(el)) {
      updateCall = createUpdateCall(el, svgPropsToUpdate);
      genericSelectAndModify(this.svg, el, updateCall, transitionTime);
    }
  });

test #1 -- click to change the date slider slightly. I am trying to stay close to max-scale of 05-08.
note the console measurements then let it settle
test #2 -- click the end of the scale, in an attempt to return to 05-08
note the console measurements then let it settle
test #3 -- click a short vertical line at bottom-left with "descendents: 5305"
note the console measurements then let it settle
test #4 -- click the same short vertical line to return to the base layout (then check that "Reset Layout" has been greyed out)
note the console measurements then let it settle

total time in modifySVG through all test steps:
1692ms master
1419ms mergeDiets
master took 20% more time in modifySVG. But the sample is not large enough for this to be significant. I need a better way to time this.

[rleir]

mergeDiets: clear cache and hard reload
       drawBranches (#1) took 656ms.
           drawTips (#1) took 244ms.
phyloTree render() (#1) took 1046ms.
total                        1946ms.

master: clear cache and hard reload
       drawBranches (#1) took 752ms.
           drawTips (#1) took 304ms.
phyloTree render() (#1) took 1206ms.
total                        2262ms.  16% more

[rleir]

A simple performance test:

mergeDiets: clear cache and hard reload   ( data: Global, in a fresh tab, try1)
       drawBranches (#1) took 576ms.
           drawTips (#1) took 388ms.
total                         964

mergeDiets: clear cache and hard reload   ( data: Global, in a fresh tab, try2)
       drawBranches (#1) took  580ms.
           drawTips (#1) took  400ms. 
total                          980

mergeDiets: clear cache and hard reload   ( data: Global, in a fresh tab, try3)
       drawBranches (#1) took 567ms. 
           drawTips (#1) took 367ms.
total                         934    avg 959.3

------
master  : clear cache and hard reload ( data: Global, in a fresh tab, try1)
       drawBranches (#1) took  599ms.
           drawTips (#1) took  412ms.
total                         1011

master  : clear cache and hard reload ( data: Global, in a fresh tab, try2)
       drawBranches (#1) took 601ms.
           drawTips (#1) took 412ms.
total                        1013

master  : clear cache and hard reload ( data: Global, in a fresh tab, try3)
       drawBranches (#1) took 589ms.
           drawTips (#1) took 397ms.
total                         986      avg 1003.33333333333  4% more

@jameshadfield James: this PR is now visually the same as master, the tip borders are no longer black but have colours like master. When comparing performance numbers, note that the previous ones had double-counting because render includes drawBranches and drawTips, sorry!
There are a few more things we could do beyond this PR to speed up drawing tips and branches, but at the moment other issues might be more important, what do you think?
cheers -- Rick