Request for performance audit

[jameshadfield]

This issue describes a general request for a performance audit of auspice, due to the (wonderful!) contributions being offered by the open source community 🎉 (I'm happy for this issue to be broken into separate issues if needed.)

Background

I've checked on performance somewhat regularly over the past few years. The main bottlenecks are exposed during animation which requires multiple tree traversals, expensive d3 updates & SVG rendering. Please also see #702 for some possible improvements which are related to this issue.

This issue calls for an audit of performance and creation of issues detailing bottlenecks and areas which can be improved.

Notes

• There are JS timing functions throughout the auspice codebase, which are by default removed at compile time. Run auspice build --includeTiming to keep them in and then get logs of the JS bottlenecks 😄
• I think there are some unnecessary react-component updates which I believe can be identified and removed.
• Comments related to available improvements in SVG/d3 code are welcome, as are thoughts on whether we should move away from SVG. I note that painting is a big bottleneck in my testing, but I am unsure how to improve this.
• Ideally, trees will be performant up to c. 5k tips, but this is a bit of a guess (they become slow currently at around 2k tips on my laptop). Display of more tips than 5k is a problem to be solved using innovative visualisation rather than improved JS speed!

[avin-kavish]

I'm working on this.

[rleir]

Hi Avin, You probably already looked at
https://nextstrain.org/charon/getDataset?prefix=/ncov/global
It loads 1.5M JSON information,
the request headers include Accept-Encoding: gzip, deflate, br
the response headers include Content-Encoding: gzip
but it took 36 seconds to download this item.
1/ does it really need to be this big? Some citation info could be lazy-loaded. Only data which is immediately needed for the graphics needs to be in this JSON, so the first paint could be quick.
2/ is the charon server overloaded? That is understandable.
I hope this helps
cheers -- Rick

[avin-kavish]

No longer working on this :P

[rleir]

Hi James @jameshadfield Just a question: the server (charon?) would not be creating this JSON at response time from a dadtabase would it? A cache would normally respond faster.
Thanks
Rick

[tsibley]

@rleir

it took 36 seconds to download this item.

I think the bulk of this must be your connection speed. I'm consistently getting in the 1-1.5s range here. The /charon endpoint does add some overhead compared to the upstream origin CDN (https://data.nextstrain.org/ncov_global.json), which times in at 0.6-1.2s for me.

[rleir]

@rleir

it took 36 seconds to download this item.

I think the bulk of this must be your connection speed.

So true. I am your worst case (best test?) with 3M down, 1M up (much slower during pandemics). I hope you don't mind me saying that the data file needs to diet. Are all the data fields needed? I will check.

July 7: sorry for the delay. The diet is not a trivial change due to the spec for the data structure. The next experiment if I get back to this would be to temporarily set the spec aside and use a simple data format. Code changes would be on server and client.

[rleir]

Hi James @jameshadfield Having an i5 CPU from a few years back, I am also your worst case for CPU cache.

• L1 256 kB
• L2 1024 kB
• L3 6144 kB

Performance test case:

• http://localhost:4000/ncov/global
• data up to May 8

CSS on each tree tip (right click -> inspect):

• pointer-events: auto;
• visibility: visible;
• fill: rgb(204, 204, 204);
• stroke: rgb(162, 162, 162);
• stroke-width: 1; moved
• cursor: pointer;

After a small code change the CSS on each tip is less (for 2000 tips, this is a big deal)

• visibility: visible;
• fill: rgb(255, 61, 45);
• stroke: rgb(220, 48, 36);
• cursor: pointer;

The visual difference is minor. The memory consumption is changed, the standard auspice shows (Shift-esc):

• memory footprint 543M
• javascript memory 124M(118M live)

After the patch:

• memory footprint 257M
• javascript memory 52M(49M live)

Quite a difference. And the tree feels faster. I am suspicious and will test again to check. Also I will do a PR just so you can see the visual difference. cheers -- Rick

[trvrb]

Interesting. I think most of this is with SVG performance rather than Mb of data in the JSON (but I bet we could save some space by reducing significant digits, ie "div": 8.997285936838663 to "div": 8.997).

But slimming down SVG without loss of aesthetics is a good direction.

[tsibley]

One way to slim down the SVG without any visual change at all is to stop using inline attributes for static values that could instead be applied by tag or .class rules in a stylesheet.

[rleir]

@tsibley Yes. I took a first stab at this in PR #1128 but I am new to that part of the code and the PR needs to be improved. By the way, when running the #1128 build the console gives me

Timer phylotree.change() (# 420) took 1ms. Average: 23ms.

Also, the tips have a border colour which looks nice when inspected closely but could be a uniform white or black. The border colour is needed so you can see a tip properly when it overlaps another tip of the same colour, but white or black would be more effective than the current colour. That could be demonstrated by another experimental PR.

[rleir]

The memory numbers I reported previously are wrong. Memory footprint changes greatly over time, and starts much higher then drops after garbage collection. I hesitate to report numbers now, other than to say that my patched build is taking approximately the same memory as the standard build.

[eharkins]

For an example case of where it could be good to use PureComponent to improve performance while allowing for needed updates, see #1176 , specifically:

maybe there is a way we could get the best of both worlds. For example, by PureComponent to make a shallow comparison of props and state.

It would be good to think about this at a high level (not limited to that example) to come up with an approach to this problem which uses React tools consistently, or as @jameshadfield put it:

Overall, yes, I think it would be better to use PureComponent in some places. For me, there’s a lot of improvements to be made in terms of speed, and I think that we should investigate (via react dev tools etc) to identify the places to improve then implement a solution, rather than just switching things to PureComponent without flagging them up first. So for the color-by component, we’d have to revisit what bits of state we’re deliberately ignoring changes to in the should component update block to see whether PureComponent will give us the performance wins that it does now.