solid icon indicating copy to clipboard operation
solid copied to clipboard

Stores hold onto unnecessary memory indefinitely in some cases

Open fabiospampinato opened this issue 2 years ago • 3 comments

Describe the bug

Some internal metadata nodes are never cleared up, so they keep consuming memory indefinitely, even if nobody needs them anymore.

Your Example Website or App

https://playground.solidjs.com/anonymous/b8626377-1d8a-4535-9aed-3bdb438b4e34

Steps to Reproduce the Bug or Issue

  1. Uncomment only the Solid code.
  2. Take a heap snapshot
  3. Uncomment only the equivalent Oby code.
  4. Take a heap snapshot
  5. Notice how the Solid code is consuming way more memory, even though the user is basically doing the same thing in both.

To magnify the effect the counter can be increased.

Expected behavior

Solid's stores should clean up after themselves, and shouldn't consume significantly more memory than Oby's stores.

Screenshots or Videos

No response

Platform

  • OS: macOS
  • Browser: Chrome@latest
  • Version: 1.6 (I think? The one in the playground)

Additional context

No response

fabiospampinato avatar Feb 15 '23 22:02 fabiospampinato

also maybe related https://playground.solidjs.com/anonymous/ef9cc6ff-5bd1-4e39-a7a2-a1c89a98f2e2

deleting nodes, generate signal internally, even for nodes that were never read.

if not, maybe need its own issue,

LiQuidProQuo avatar Feb 15 '23 22:02 LiQuidProQuo

These issues are tricky because in my opinion, it isn't clear when if every something should stop tracking. Obviously we could look if is being tracked at the time but we aren't likely to check when things stop being tracked. This has basically nothing to do deletion or not as deletion and setting could be tracked. And we can track keys that aren't present yet.

Rather this asks the question whether we should only hold signals while store nodes are being tracked. This is a much more interesting question. My gut is that more negative than the current behavior for the vast majority of cases. This feels more like a discussion question to be fair.

@LiQuidProQuo you are right that is an implementation detail currently because of some changes we needed to make for async consistency it probably could be its own issue but it also unlikely to be fixed with the current approach and again is a very narrow case.

ryansolid avatar Feb 22 '23 20:02 ryansolid