lit / lit

Lit is a simple library for building fast, lightweight web components.
https://lit.dev
BSD 3-Clause "New" or "Revised" License
18.6k stars 915 forks source link

[lit-html] Memory/Handlers Leak Issues: Rendering Tree of CustomElement using only lit-html, Regression? #3065

Open fopsdev opened 2 years ago

fopsdev commented 2 years ago

Hi there

I'm rendering trees of custom elements which itself render themself using lit-html.

Simple repro here: https://github.com/fopsdev/OvlReproMemoryLeak

Please open up devtools when you check out the repro.

It works as expected with lit-html 1.1.2

sorvell commented 2 years ago

Here's a playground made from the repo, and running outside the playground here.

Does this reproduce the issue? If so, can you give more explicit step-by-step instructions for how to reproduce it?

Force a GC (Garbage Collect)

This can be done in the Memory tab by click the trash can icon.

Check the handlers

What does this mean?

Thanks.

jridgewell commented 2 years ago

The issue is the global TreeWalker, which holds onto our template elements during Template's constructor and TemplateInstance's _clone. We need to reset the walker.currentNode to the global document to ensure the fragments are not leaked. The not-so-bad part is that only one tree could be leaked at a time, and it'll only leak until the next time something is _cloned.

fopsdev commented 2 years ago

@jridgewell Thanks for catching up on this. This thing drove me close to crazy since i didn't had much experience hunting memory leaks. @sorvell With Handlers/handlers i mean the Nr. of handlers in the Performance Monitor inside Chrome Devtools Btw. the playground doesn't work here (currently?)

justinfagnani commented 2 years ago

Thanks for looking into this @jridgewell !

The one thing I wonder about, if the global tree walker is the cause, is how that would show up as a leak that's noticeable. Is it just that one tree has some handlers and we expected them all to be release after GC, or was @fopsdev seeing a continuous increase in objects/memory?

sorvell commented 2 years ago

Here's what I see when testing the playground (look like the stand alone link is not stable so I pop the iframe into a new tab to test). The downward spike at the end corresponded to pressing the garbage collect button in the dev tools. This seems to indicate that this exact repro is not demonstrating a problematic leak that could grow in an unbounded way.

Screen Shot 2022-06-23 at 2 21 44 PM
jridgewell commented 2 years ago

It shouldn't be unbounded, only a single detached DOM tree can be held by the walker at a time. So it'll be whatever was last instantiated into the document with _clone (or whatever was last invoked with html tag if nothing has been inserted into DOM).

fopsdev commented 2 years ago

thanks so far for the insights fyi: i'll spend some time with this issue most probably on sunday

fopsdev commented 2 years ago

Hi again

Had some time already to check this out :)

Having those retainers is really adding up in my full blown framework.

I've changed the repro so it acts closer to my framework: https://user-images.githubusercontent.com/5448300/175766710-9ff07375-5dda-485a-a012-cdf4095a656a.mp4

as i said with lit-html 1.1.2 it works: https://user-images.githubusercontent.com/5448300/175766863-139f76bb-6d5a-465d-975a-ff8ba8ad6d95.mp4

hope that helps

AndrewJakubowicz commented 1 year ago

This issue may be resolved by https://github.com/lit/lit/pull/3888. The tree walker no longer holds onto the last DOM tree.

Is your issue resolved with lit-html@2.7.4? Thank you!