preact-devtools icon indicating copy to clipboard operation
preact-devtools copied to clipboard

Memory is leaked when a component is updated - Simple Reproduction

Open sschultze opened this issue 1 year ago • 6 comments

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 avatar Mar 29 '23 11:03 sschultze

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.

sschultze avatar Mar 29 '23 11:03 sschultze

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

marvinhagemeister avatar Mar 29 '23 12:03 marvinhagemeister

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?

sschultze avatar Mar 29 '23 13:03 sschultze

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 avatar Mar 29 '23 15:03 marvinhagemeister

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 avatar Mar 29 '23 16:03 sschultze

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.

sschultze avatar Mar 29 '23 18:03 sschultze