fix(runtime): clear up rootAppliedStyles

[christian-bromann]

What is the current behavior?

There seems to be a memory leak within Stencil that causes performance problems as documented in:

• bug: light dom custom element with comment node causes retainer ionic-framework#28189
• bug: certain combinations of ionic components cause memory leak  #5181
• bug: conditional rendering with ionic component causes memory leak #5172
• bug: Potential Memory/Resource Leak #3607
• and possibly bug: Deleting Element with @State causes memory leak when used in componentDidLoad lifecycle hook #3158

It turns out that Stencil doesn't properly clear up its hostRefs global when elements are removed from the DOM. Given that the hostRefs global is never garbage collected, all its value will remain in memory which requires us to do some manual cleanup. Furthermore, we have been using references in another global variable called rootAppliedStyles which also never gets cleaned up.

What is the new behavior?

This patch enhances the disconnectedCallback hook to clean up detached nodes from the hostRefs and rootAppliedStyles maps.

Documentation

Does this introduce a breaking change?

• Yes
• No

Testing

I am investigating if we can add some memory checks through Puppeteer and Chrome Devtools.

Other information

A development release is now available at @stencil/[email protected]. If anyone is able to help validate the memory leak fix, your assistance would be greatly appreciated.

[christian-bromann]

I've done some further testing on the following reproduction case: https://github.com/alicewriteswrongs/stencil-1079-memory-leak-example

Behavior before patch:
While the performance monitor showed signs that GC picks up detached DOM nodes:

It is clear that there is a minimum amount of nodes that isn't being picked up. This can be proven by logging the hostrefs which show an increasing number of entries:

Behavior after patch:
The saw curve is now much more clear to determine:

and hostrefs remain the same at all times:

[christian-bromann]

Investigated another reproduction case: https://github.com/joewoodhouse/stencil-modal-memory-investigation

I made the same observations: the performance monitor and DOM node curve looked the same for the version with as well as without patch, however without the patch the number of nodes in the hostref WeakMap would continue to increase:

while in the patched version, it would remain around 32 nodes.

[christian-bromann]

Further investigations have shown that cleaning up the hostRefs Weakmap is not a good idea because we never know when or if we may re-attach a node back to the DOM again. Some of our e2e tests have shown regression in this area. I definitely believe that cleaning up the rootAppliedStyles has a positive effect and reduces the amount of detached nodes. However I can still observe nodes lingering around so my investigation will continue.

[gnbm]

Great work and analysis @christian-bromann 🚀

[seregindev]

Hey @christian-bromann! Great work, and many thanks to you for that. Do you have a new version with the last changes that I can use for testing?

[christian-bromann]

@seregindev I just published a new dev version @stencil/[email protected], please give it a try and let me know if you see any improvements.

[christian-bromann]

🙈 thanks, great catch! Pushed a new dev build @stencil/[email protected]

[christian-bromann]

Updated: @stencil/[email protected]

[christian-bromann]

Last update: @stencil/[email protected] .. this is ready for merging now. I reviewed the fix in 3 reproduction cases:

• joewoodhouse/stencil-modal-memory-investigation
• jameson-w-taylor/modal-detached-nodes
• alicewriteswrongs/stencil-1079-memory-leak-example