CPU and Memory Usage

[reustle]

We're having quite a but of memory usage (leaks) and cpu usage over time. Would love some help debugging and optimizing, as unfortunately I'm quite swamped with work right now. We can discuss anything related in this card. Thank you!

[a-ogilvie]

@reustle Can you share the results of any investigation you've done so far?

[reustle]

Thanks for reaching out, I haven't dug too deep but here's what I know:

• Sometimes we see 'memory usage limit reached' on sentry errors list for a select few users, probably who've left the page open for a long time. We've added an auto page refresh every few hours for the time being. Likely some sort of memory leak

• The initial page load has become significantly slower. Map, tons of charts / graphs, etc is likely the case of this. It would be nice to ensure that there aren't any inefficient calculations happening at least. We then need to revisit the idea of removing many of the charts or simplifying them. If it's slow on my laptop, its definitely slow on phones.

Feel free to chime in @liquidx

[liquidx]

I'm guessing there might be something we're doing inefficiently with rendering the charts.

I suspect processing a 100~ rows of data isn't really the bottleneck on the browser. But we do get out of memory errors, which may mean that there's a memory/reference leak somewhere we're not catching when the page is loaded for a long period of time.

[l15n]

Here are some memory usage snapshots on the same page over time.

Initial load:

About 20 minutes later:

The clear big change here is an increase in memory usage from ArrayBuffer objects. This strongly suggests that we're somehow failing get rid of old charts after data is updated (I think d3/c3 use ArrayBuffer internally)

[jonathanpperry]

Hi Leonard, out of curiosity what program did you use to capture these memory usage snapshots?

[l15n]

Hi Leonard, out of curiosity what program did you use to capture these memory usage snapshots?

It's the memory tool in Firefox Developer Tools, built into Firefox.

(My preference is to use Chrome for the Performance Profiler, and Firefox for everything else)

[l15n]

First attempt at perf improvements in #240.

Also, after finally reading through all the code (not a fan of the file structure 😢 ), I do think that using React/Preact or Vue would be worthwhile because there's a lot of rendering code now. React and Vue, although they might seem heavyweight, they do enforce a clear structure, and through that structure they result in a more performant app through how they control rendering. I'd chose Preact (basically a lightweight version of react), actually.

Of course, there's going to be a switching cost that someone has to pay if we do choose to go that way :wink:. Probably won't be that bad though.

[mertd]

If rerendering is the main culprit, porting the app to a framework that takes care of updating the DOM (like the ones mentioned by @l15n) instead of us changing it directly should help.

[reustle]

+1 or React / Preact. As un-fun as the file structure might be, it is definitely aligned with those types of projects, and agree that simplifying the render logic would be a good step forward. A good first bite would be to convert some of the very basic components (Last Updated time, Lang Picker, etc) into components. Thanks everyone!

[l15n]

Now that the macrotasks have been broken up, I've been able to take a closer look at what's actually happening. It turns out that the biggest culprit is c3: it has been written such that any customizations (e.g. for labels, colors, etc) actually don't happen offscreen, and trigger redraws while drawing it out.

Although React might help in the long term, the biggest performance term gain will be in replacing c3 with a charting library that builds the charts out of the DOM.

[reustle]

Good observation, one thing I noticed when looking into a few charting libraries before was that many of them are "react" components etc such as Recharts. I wonder if this could be a "2 birds with 1 stone" scenario, if were to dip our toes into preact using an optimized charting lib in that way.

[l15n]

In order to demonstrate this, here's a quick walkthrough of how the rendering of the Trajectory chart changes as options are removed, and then added back.

Baseline

In the existing implementation in production, the task takes over 1s to draw the chart.

Minimal implementation

I then removed all the inline functions: specifically, for label, color and tooltips. Now it only takes 181ms to render

Restore tooltips

Since tooltips aren't actually visible, it seems like there is negligible cost. 185ms

Colors

The function for setting colors (i.e. provisional values) results in task time of 286s ms

Labels

The prefecture labels are the most costly to render. These are the labels for Tokyo, Kanagawa etc drawn inline. Now takes 777ms

You can clearly see the functions for redrawing are causing lots of tiny changes in Layout (dark purple) in the flame graph.

[l15n]

Good observation, one thing I noticed when looking into a few charting libraries before was that many of them are "react" components etc such as Recharts. I wonder if this could be a "2 birds with 1 stone" scenario, if were to dip our toes into preact using an optimized charting lib in that way.

Yep, that's another option that I'm looking into. With Preact, though, we will need to check for library compatibility.

Some candidates that I have in mind are:

• canvas.js: https://canvasjs.com/ - been around for a while, uses canvas.
• rechart (as you mentioned, requires react) https://recharts.org/en-US/
• nivo (also d3 + react) - https://nivo.rocks/components
  • This has storybook support so you can do prototyping
  • seems to have a good perf story, with support for drawing to canvas as well

As with all things like this, it would be good to build a prototype or two first! I'm personally intrigued with Nivo, so will probably give it a go when I get some free time.

[reustle]

Thanks @l15n ! Linking to recharts here as I think there was a copy-pasta error above https://recharts.org/en-US/

I personally tend to like the styling / shapes of recharts more, and fonts can likely be changed. I totally agree with building a prototype first.

[dlinx]

One root cause I found for the performance is the map. If we use static map, then there will be a lot more impact on performance.

[reustle]

Hey @dlinx, thanks for looking into this. We totally agree, a static map would be a great improvement https://github.com/reustle/covid19japan/issues/39

[liquidx]

I ran lighthouse (Chrome Dev Tools) on the site (both / and /embed) and it looks like the performance is not good both with and without the charts.

That might be a good list to get started on. I'm not attaching the whole report as you can run it inside Chrome and get the full drilldown.

[dlinx]

@reustle Any updates about implementing framework? I think its better to switch to react as the application in VanilaJS becoming much difficult to maintain.

[reustle]

@dlinx Yes we are likely going to go with something like react or preact, but are still sorting out the details. It will indeed help better structure the codebase.

My vote would be for normal ol' React.

[dshlass]

Is React still being considered? I'd love to help out with the rewrite

[reustle]

It is definitely being considered, but there is nobody championing it at this time. It should definitely be done in many small steps.

[dshlass]

I've been experimenting with the KPI section using nextjs and server side rendering. The performance so far is great but I'm not sure what it will be like once I move onto the prefecture tables/the map. Will continue to toy around with it and report back once I've experimented some more.

[reustle]

Going to close this for now, as a fresh audit likely needs to be done in the future, as the codebase has changed quite a bit over time.