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

Memory is leaked when a component is updated - Simple Reproduction #473

Open
sschultze opened this issue Mar 29, 2023 · 6 comments
Open

Comments

@sschultze
Copy link

Describe the bug

Memory is leaked when a component is updated.

My Electron (happens in Chrome also, see below!) application has a progress display (status text showing transferred bytes, progress bar) which updates very often. As Electron has the V8 Memory Cage enabled since version 21, limiting the memory to a few gigabytes (they say 4 GB, but it is 3 GB in my tests), the application frontend goes away after a while (leaving an empty BrowserWindow).

After spending a lot of time tracking down this issue, I was able write a simple reproduction - see below - which fills up the memory very fast in Chrome (see Chrome DevTools -> Memory).

To Reproduce

Use a current version of Chrome.

import { render, useState } from 'preact/compat';

// The only purpose of ChildComponent is to create a complex tree, eating up memory fast.

interface ChildComponentProps {
    readonly level: number;
}

function ChildComponent({ level }: ChildComponentProps) {
    return (
        <div>
            <div>{new Date().valueOf().toString()}</div>
            {level < 4 && Array(3).fill(0).map((_, index) => (
                <ChildComponent key={index} level={level + 1} />
            ))}
        </div>
    );
}

function RootComponent() {
    const [counter, setCounter] = useState(0);
    setTimeout(() => setCounter(counter + 1), 0); // simply force updating all the time
    return (
        <div>
            <span>{counter.toString()}</span>
            <div style={{ display: 'none' }}>
                <ChildComponent level={0} />
            </div>
        </div>
    );
}

const rootElement = document.getElementById('root');
render(<RootComponent />, rootElement!);

Expected behavior

Memory usage should remain quite constant. It should definitely not lead to an out-of-memory error.

@sschultze
Copy link
Author

Just in case the suspicion comes up, I am not actually updating the state with setTimeout(..., 0) in my application. I am using useSyncExternalStore to subscribe to progress changes in the backend. But using setTimeout made the reproduction simpler.

@marvinhagemeister
Copy link
Member

marvinhagemeister commented Mar 29, 2023

I gave this a go and limited the counter to 10000 iterations. From looking at the profiler tab memory seems to be freed correctly by Chrome when in incognito mode. But when DevTools is active it doesn't seem to free memory in the same way. Do you happen to have the DevTools bridge active?

With DevTools:

Screenshot 2023-03-29 at 14 46 26

Without DevTools:

Screenshot 2023-03-29 at 14 42 51

@sschultze
Copy link
Author

Thank you @marvinhagemeister ! I included performance.memory.usedJSHeapSize in the UI in order to not having to open DevTools. Same error, memory fills up very fast.

I didn't manually include the DevTools bridge in the reproduction app, but prompted by your hint, I looked into the generated source code and it seems that the Vite development server injects it:

import { addHookName } from "/node_modules/.vite/deps/preact_devtools.js?v=eaf7af1c";

When I build the reproduction app for production (vite build), the problem does not occur and memory usage stays under 10 megabytes.

So should I learn from this that I shouldn't import preact/devtools in the production version of my actual app? Or is this just a bug in the bridge that is likely to be solved?

@marvinhagemeister
Copy link
Member

Awesome, that narrows it down! I'd definitely count a memory leak as a bug and something to be addressed on the DevTools side of things.

@marvinhagemeister marvinhagemeister transferred this issue from preactjs/preact Mar 29, 2023
@sschultze
Copy link
Author

Great! Until this is fixed, is there a way to conditionally include the bridge (I mean preact/devtools) or, as an alternative, disable it based on some flag after it is loaded?

@sschultze
Copy link
Author

If I import preact/devtools in the reproduction app (first import statement, as suggested in the docs) and build it for production (vite build), the memory usage also stays low.

I should have realized this earlier, sorry for that.

Fixing a leak that only occurs in development mode (or even only in combination with the Vite dev server, I don't know) is probably low priority.

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

2 participants