Performance degradation (page freeze) during re-rendering of LayerCake component

[dlebech]

Hi @mhkeller, thank you for a nice library 🙂

What is it

We recently noticed a quite severe performance problem that occurs when a LayerCake graph is re-rendered, e.g. as part of a reactive DOM update.

How to reproduce

Svelte Repl: https://svelte.dev/repl/84391ac6b7ea4a628cdbaefb142c6e3a?version=3.55.1

• Clicking the "Sort by First name" will initially seem to work ok.
• After a few clicks (3-5 clicks in my case in both Chrome and Firefox), the rendering gradually slows down and it takes multiples seconds for it to re-render.
• The page eventually becomes completely unresponsive.

Note, if you remove the <BarChart /> component from the list and replace it with text, the sorting continues to be fast, so the performance issue is definitely in the LayerCake library -- furthermore, making an empty <LayerCake></LayerCake> component also shows the problem -- I just used a bar chart to have something visual to look at.

Discussion

Without looking at the code, the behavior at least looks a bit like a recursive call that keeps getting deeper and deeper.

Perhaps there is a circular dependency (maybe some reactive statements) or function call somewhere that goes a bit out of control after a few re-renders?

Thank you for your time.

[mhkeller]

Thanks for the detail and the reproduction! I did an initial look. I'm not sure why this fixes it but if you remove the keyed each block and make the data assignment inside of BarChart.svelte reactive (which I think is a good idea anyway), it works fine: https://svelte.dev/repl/374b147c2baf43328d6b276421f11a6c?version=3.55.1

I'm not sure what the keyed each block is doing that slows it down but since it works inside of a normal loop, I'm not sure the issue is related to a leak or recursion inside the library – that should be showing up regardless, no? I think looking into what the keyed each block is doing internally in svelte would be a good place to start and then see if there is a bug when adding too much complexity inside of those loops. I'm not yet sure this is a layer cake bug or a svelte one.

[mhkeller]

It reminds me of behavior using backbone.js where you had to manually destroy models and views. If you forgot to destroy a view component, its model state would persist in memory and every time you did something, it was simultaneously updating all previously created views. perhaps the keyed each block is persisting those states somewhere and when the array is re-rerun, all the previous ones are updating.

I kind of tested this by increasing the number of items from 10 to 100 and it slows after many fewer button clicks. In my non-keyed version, you can increase the number to 1,000 and it works fine. This makes me think it's something about persisting

[dlebech]

Thank you for your quick reply.

I'm not sure why this fixes it but if you remove the keyed each block and make the data assignment inside of BarChart.svelte reactive (which I think is a good idea anyway), it works fine

Thanks for the suggestion for a workaround -- it will do ok in the specific case I have in our app, since I technically don't need the keyed each in the specific case where I'm using a chart in a keyed each 👍

I'm not sure what the keyed each block is doing that slows it down but since it works inside of a normal loop, I'm not sure the issue is related to a leak or recursion inside the library – that should be showing up regardless, no?

I'm not familiar enough with how Svelte deals with keyed each, but there must be some complexity in LayerCake that it doesn't work well with.

I updated the above repl with a component "SimpleLayer" that sets a context with a Svelte store and a "SimpleChartComp" that reads from this context-store and creates an SVG bar chart from its value-- in other words: Conceptually a bit similar to how LayerCake works.

This component can be used in a 1000-row keyed each and doesn't seem to slow down after re-sorting multiple times.

I'm not saying it's a bug -- I think you have very valid points -- just adding more context (pun intended) 🙂

[mhkeller]

Awesome thanks for that. I'll have to see which parts of the library end up being too complex for it. LayerCake itself is just a context with some stores, really, so I'll be curious what combinations of things get too much to deal with. If you find out anything else let me know.

[mhkeller]

The issue is with Svelte. It happens when you have an element inside of a keyed each block that binds to clientWidth or clientHeight. Here's a reproduction: https://svelte.dev/repl/80c226b9830f4dfa8de5e70e7c83f7e0?version=3.55.1

I can make an issue on Svelte

[dlebech]

@mhkeller Thanks for looking into it, and nice find on the Svelte issue 🎉

[mhkeller]

Thanks for bringing it up @dlebech! Following some of the issues linked on the svelte issue I made, I wonder if implementing this resize observer action would be a good fix in the meantime: sveltejs/svelte#7583

I'll need to understand any potential drawbacks. It is not supported in IE11, if that is a concern... It would be nice if it could fall back to the slower behavior in that environment...

[dlebech]

I saw your discussion in #114 and linked issues, and it sounds like a good idea to just wait for it to be sorted by Svelte core. I appreciate you taking the time to dig into it -- maintaining a library on the side requires laser-focus on the things that are important 👍

As a sidenote, I will definitely be more careful using bind:clientWidth in other code going forward -- I had no idea that an iframe was being used for this -- or at least I didn't connect the dots when I saw iframes being created next to LayerCake graphs