microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.52k stars 28.99k forks source link

Detached DOM nodes memory leak #146841

Open rebornix opened 2 years ago

rebornix commented 2 years ago

The Edge Dev Tools now offers Detached Elements tool to help detect memory leaks. The tool is a good addition to the memory heap tool but it's analyzing from a different angle. @sbatten and I spent some time digging into this tool and fixed memory leaks in the ActionBar successfully. Here is how we tackle the leaks in action bar:

Turned out we register a global DOM listener for every ActionViewItem and store it in ActionBar disposable store. When the ActionViewItem is disposed, the DOM listener will be not disposed/cleared until the ActionBar is disposed.


With the ActionBar leaks fixed, there are still 30+ detached DOM nodes not GC'ed. Not every one is necessarily memory leak (for example, if you are using ListView, it will hold refs to template DOM nodes) but some of them are duplicated elements.

jrieken commented 2 years ago

👏 wow, this is a pretty cool tool

isidorn commented 2 years ago

@rebornix super cool. And thanks for fixing the ActionBar bug which I probably introduced originally :) fyi @hediet for AccessibilityHelpWidget

bpasero commented 10 months ago

Using this tool on insiders.vscode.dev with Edge, I see a detached HTMLDivElement which seems to be the monaco editor every time I switch between 2 tabs (which I find surprising because we do reuse the same editor instance in the same editor group):

image

Does someone have an understanding where these are captured exactly?

image

//cc @hediet @alexdima