preactjs / preact

⚛️ Fast 3kB React alternative with the same modern API. Components & Virtual DOM.
https://preactjs.com
MIT License
36.35k stars 1.93k forks source link

Rendering just gets slower over time #4383

Closed SuperDisk closed 1 month ago

SuperDisk commented 1 month ago

Describe the bug It seems that as time goes on and things get rendered repeatedly, performance just slows down to the point where rendering takes much, much longer than it did at the start. I experienced this in one of my projects, and when I switched to React, the performance issue went away, which implies it's Preact-specific.

To Reproduce

https://codesandbox.io/p/devbox/preact-bug-cr5gfc?file=%2Fsrc%2FApp.tsx%3A21%2C1

Steps to reproduce the behavior:

Press Re-render 1000 times to do some re-rendering. At first it's quite fast, and then slowly but surely it bogs down to the point where it's taking a massive amount of time to increment the counter.

I can reproduce the bug in Firefox 124.0.2 (64-bit), but not on Chromium Version 124.0.6367.155 (Official Build) snap (64-bit)

Expected behavior Performance should remain at the same level as when the app starts up. This seems like a pretty severe issue to me.

JoviDeCroock commented 1 month ago

Hmm, that's odd! Mind trying something like

import { options } from 'preact'

options.debounceRendering = queueMicrotask

Also try that with requestIdleCallback and setTimeout.

SuperDisk commented 1 month ago

Just tried it, the same problem still seems to occur using setInterval, requestIdleCallback, and setTimeout.

JoviDeCroock commented 1 month ago

For me it works correctly on both FF and Chrome 😅

EDIT: you didn't do what is in the code-snippet though 😅

SuperDisk commented 1 month ago

EDIT: you didn't do what is in the code-snippet though 😅

What do you mean? I put that code snippet in and tried all the variations. Then I removed the snippet, so that's why it's not there when you click the link.

JoviDeCroock commented 1 month ago

Gotcha, the reproduction changed all the time in a few different ways 😅 either way mind doing

import { defineConfig } from "vite";
import preact from "@preact/preset-vite";

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [preact({
    prefreshEnabled: false
  })],
});

as it might just be prefresh holding onto vnode refs - in my tests this fixed it

SuperDisk commented 1 month ago

I just tried that as well-- it does seem to speed things up a bit, but by the time it reaches 1000 it's definitely slowing down.

JoviDeCroock commented 1 month ago

Could you do a memory dump because for me there's no more leak with that disabled

SuperDisk commented 1 month ago

image image

It's still leaking for me-- notice it went from 85 MB to 113 MB in about 30 seconds.

kitten commented 1 month ago
Screenshot 2024-05-14 at 13 17 49 Screenshot 2024-05-14 at 13 25 43

Not exactly what I've been observing. It may obviously take a bit of time to track down where the leak is coming from, especially since it seems to be isolated to FF, but on FF 126 this doesn't seem to be happening with Prefresh enabled.

Could you make sure please to reload the sandbox entirely in a new tab, if you haven't yet, enable "Record call stacks" and save two memory snapshots and attach them here please?

SuperDisk commented 1 month ago

Sure, I included 3 here: snapshots.zip

ritschwumm commented 1 month ago

if i understand the code correctly, then every click on the button creates a new setTimeout "loop" and this loop never ends - so i'd expect it to become slower with each click on the button due to the increasing amount of active setTimeouts

additionally, the setTimeout callback keeps at least setCount and probably more things alivwe in memory, so i'd expect it to leak memory, too.

JoviDeCroock commented 1 month ago

This is correct yes! I didn't look at the new code, the first version made sense as it just called setCount 1000 times 😅

SuperDisk commented 1 month ago

I modified it to count up infinitely, you only need to click it once. There's definitely a leak not related to the setTimeout/setInterval stuff, if you look at the memory dumps I uploaded they grow by megabytes infinitely. I also updated my Firefox to 126 and I'm still getting the same issue.

SuperDisk commented 1 month ago

https://github.com/preactjs/preact/assets/1688837/63990188-9844-48b1-b894-15380deea189

You can see it starts extremely fast, but by the time it reaches 1000 it's crawling.

SuperDisk commented 1 month ago

Aha, I disabled Preact Devtools and it seems the memory leak goes away. My bad for not mentioning I had that enabled, I forgot about it :(

Curious what's causing it there though.

JoviDeCroock commented 1 month ago

Both prefresh as well as the dev-tools hold onto VNodes, we are probably running into some scenario where we fail to cut the link of i.e. _parent/_children and it cant' be gc'ed.

JoviDeCroock commented 1 month ago

CC @marvinhagemeister just so you're aware of this

JoviDeCroock commented 1 month ago

Will close out this issue now as we're aware of this issue in prefresh/devtools but those are also just for development. Most of the ESM HMR tools have a fundamental memory leak https://github.com/vitejs/vite/discussions/14438 - we might be able to circumvent some of it with i.e. https://github.com/preactjs/prefresh/issues/545 but it will always be there in some way