Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rendering just gets slower over time #4383

Closed
1 task
SuperDisk opened this issue May 14, 2024 · 18 comments
Closed
1 task

Rendering just gets slower over time #4383

SuperDisk opened this issue May 14, 2024 · 18 comments

Comments

@SuperDisk
Copy link

  • Check if updating to the latest Preact version resolves the issue

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
Copy link
Member

Hmm, that's odd! Mind trying something like

import { options } from 'preact'

options.debounceRendering = queueMicrotask

Also try that with requestIdleCallback and setTimeout.

@SuperDisk
Copy link
Author

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

@JoviDeCroock
Copy link
Member

JoviDeCroock commented May 14, 2024

For me it works correctly on both FF and Chrome 😅

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

@SuperDisk
Copy link
Author

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
Copy link
Member

JoviDeCroock commented May 14, 2024

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
Copy link
Author

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
Copy link
Member

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

@SuperDisk
Copy link
Author

image
image

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

@kitten
Copy link
Contributor

kitten commented May 14, 2024

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
Copy link
Author

Sure, I included 3 here: snapshots.zip

@ritschwumm
Copy link

ritschwumm commented May 14, 2024

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
Copy link
Member

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
Copy link
Author

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
Copy link
Author

preact-problem.mp4

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

@SuperDisk
Copy link
Author

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
Copy link
Member

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
Copy link
Member

CC @marvinhagemeister just so you're aware of this

@JoviDeCroock
Copy link
Member

JoviDeCroock commented May 18, 2024

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 vitejs/vite#14438 - we might be able to circumvent some of it with i.e. preactjs/prefresh#545 but it will always be there in some way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants