Memory leak

[cynecx]

Version: 1.50.0-insider
Commit: 0ecb64a
Date: 2020-10-02T13:37:26.948Z
Electron: 9.2.1
Chrome: 83.0.4103.122
Node.js: 12.14.1
V8: 8.3.110.13-electron.0
OS: Linux x64 5.8.13-arch1-1

Steps to Reproduce:

1. Prepare a large text file (Citylots.json is ~190MB):

wget "https://github.com/zemirco/sf-city-lots-json/blob/master/citylots.json"
cp citylots.json evenlarger.json
cat citylots.json >> evenlarger.json
cat citylots.json >> evenlarger.json

2. Open evenlarger.json in vscode.
3. Scroll around.
4. Close the file.
5. Observe the memory usage through "Process Explorer".

Even after ~30 minutes, the memory usage is still high:

Does this issue occur when all extensions are disabled?: Yes

Note: This seems like a regression because I didn't had this issue before, however I don't know which insiders/stable release caused this.

[alexdima]

I found this interesting -- https://chromium.googlesource.com/chromium/src.git/+/master/docs/memory/key_concepts.md

[bpasero]

//cc @deepak1556

[cynecx]

Is there a way to take a memory/heap snapshot of the workbench process? Dev-tools's memory profiling doesn't seem to capture the workbench process where the leak occurs.

[cynecx]

Another observation is that this isn't really related to large files. This is a general memory leak. I can reproduce this by opening several 5-10MB text files and see the memory usage climbing without any reclamation being done even after closing all the editors and waiting several minutes.

Workaround: Try to reload the vscode window when you notice memory pressure on your system. This is really annoying but I guess it's better than running out of memory :D.

[paulkek]

I have also been experiencing this problem, however I am not exactly working with large files but simple 10MB plain log files. The memory usage doesn't drop after having several files opened and closed.

@bpasero Why did you mark this issue as "out-of-scope". This is clearly an issue, except memory leaks are considering okay by the vscode team.

I'd also point out that this didn't happen with earlier releases of vscode.

[mithusingh32]

This could be electronjs issue and not solely vscode and that could be why it's out of scope for vscode.

A quick test could be to create a quick json viewer using electronjs and see if the memory leak still occurs.

[cynecx]

I’d like to point out, that since last time I’ve tested it (some months ago), it seemed like a regression because both leaking and non-leaking release of vscode were using the same electron version. However, I am sorry I haven’t taken notes and can’t really remember, nor do I have the resources and time to bisect this “regression”. The least I can say is that I can still reliably reproduce this memory leak.

[00benallen]

I think it's quite irresponsible to mark this out-of-scope, this affects all of your users on all platforms.

[nico-abram]

I believe I ran into this about a year ago (#76498)

[dataf3l]

Guys,
It's up to them to decide what is the scope.
It's up to us if we decide we want to use their Software.

They have made their choice.
We make our own choices.

[dlight]

There is "Improve performance, scalability, and security of VS Code and its extensions" in the roadmap. I don't see how this issue isn't relevant for that.

[sam0x17]

Front page of HN = should probably fix hint hint ;)

[krainboltgreene]

That's not how open source software works, @sam0x17.

[gampu]

vscode is being used by many online IDE services like gitpod. Such memory issues could impact their performance.

[szszoke]

How does the memory footprint changes if you open a few files, close all of them and then open them again?

Are you getting increasing memory usage until your system is out of memory, or the second time you open the same files, the memory usage doesn't increase with the same amount as it did when you opened the file for the first time?

[romulof]

Allocating and freeing memory on demand without restrains will lead to memory fragmentation, degrading performance.

Need 100MB? Allocate it.
Don’t need anymore? Ok, keep that space, we might need it soon.

VSCode maintainers were totally right to descope this “bug”. If you desperately want that memory back, I’d recommend checking with Node/V8 devs for JS APIs to control memory management and/or garbage collection.

[ilicmarko]

Didn't test this myself but holding memory is not necessary a "memory leak". When we talk about memory allocations like malloc and free they don't "delete" the memory and return it to the OS. What they do is usually mark is as available and if the OS needs it, it can take it back.

A better memory leak would be opening and closing the same file. If you see the memory starts jumping after a couple of tries there is a memory if not it reusing the freed space.

Edit:
Sorry for duplicate explanation, so many people added a comment while I was typing 😅

[sharpninja]

VSCode maintainers were totally right to descope this “bug”. If you desperately want that memory back, I’d recommend checking with Node/V8 devs for JS APIs to control memory management and/or garbage collection.

Maybe nudge the VSCode team to embed a CLR in VSCode as an alternative to JavaScript.

[corymainwaring]

I can get this to happen with files in the current workspace and outside of the current workspace. Opening the same file, closing it, and opening it again will cause an increase in memory usage proportional to the size of the file. The memory does not get reclaimed. This continues unbounded for me.

I can get the memory to be reclaimed by opening a new workspace. I'd argue that it makes it a memory leak, since I'm testing with the same file within a single workspace, then swapping to a new workspace and immediately getting reclamation of space. Maybe that's expected behavior, but it's a bad user experience anyway if I have to keep all of my files opened all the time to avoid leaks. So far, I've only found switching workspaces to reset memory usage without restart. Reloading the current workspace does not work.

Environment:
Version: 1.51.1 (system setup)
Commit: e5a624b
Date: 2020-11-10T23:34:32.027Z
Electron: 9.3.3
Chrome: 83.0.4103.122
Node.js: 12.14.1
V8: 8.3.110.13-electron.0
OS: Windows_NT x64 10.0.18363

[cynecx]

Opening a new or restarting a workspace just circumvents the issue because it spawns a new electron renderer process which includes a new v8 instance.

I have seen this top comment on Reddit:

Visual Studio Code could even be intentionally holding onto the freed memory for potentially improved performance in case the memory is needed again.
A better test for a memory leak would be to repeatedly open and close the same file to see if the memory usage continues to jump each time. I just tried that with your example file, on both VS Code 1.49 and the latest VS Code 1.51, and confirmed that continually opening and closing evenlarger.json does not increase the amount of memory used. This confirms that each time you open the file, Visual Studio code is reusing the previously allocated memory, so it is not a memory leak.

I have also tried different files with different and I can still confirm not seeing memory being reclaimed, which means that memory buffers aren’t reused. However that does not mean that the file hasn’t been cached.

That spawns another question whether v8 or vscode is caching file contents.

Perhaps I’d like to mention this once again, this didn’t happen before with the same electron version iirc and different vscode releases. So the question is, did vscode introduce some sort of file caching? But that wouldn’t explain the behavior I am seeing above.

Edit:

I’ve tried to use the integrated profiler to take a heap snapshot, however it doesn’t seem to capture any of the memory related to the text buffer. Even with an opened ~300MB file, devtools reports a total heap size of ~60MB.

[alexdima]

Just wanted to add a note here for clarity:

I have investigated this back in October, but I did not find a logical leak in our code. We took two heap snapshots, one taken before opening a very large file and the other one taken after closing the file. The logical JS memory usage did not grow (according to the Heap Snapshots). That is why I linked to https://chromium.googlesource.com/chromium/src.git/+/master/docs/memory/key_concepts.md , since it appeared to me that we free the memory from JS, but v8/chromium does not release the memory back to the OS. AFAIK that is typical behavior for many garbage-collected runtimes. So we simply stopped our investigation since this behavior does not come from our code-base and the behavior was different across Electron versions (with the same vscode code).

[alexdima]

Actually, just for completeness sake, I have tried one more time to reproduce and this time found a logical leak. The thing is that the leak only reproduces out of Insiders or Stable and not when running from source. This is due to the difference in product.json and the presence of file based recommendations in the stable/insiders builds. Back in October, I only tried to reproduce running from sources and believed the issue is about the difference in the memory usage reported by the heap snapshot and the OS and the issue was about how v8 is freeing memory from the OS.

But the following does show a clear leak:

There is a listener whenever a text buffer is created that is not disposed around here -- https://github.com/microsoft/vscode/blame/c4b7d109123ca612cbe3d40e9c67575a145ec020/src/vs/workbench/contrib/extensions/browser/fileBasedRecommendations.ts#L152-L155 .

@sandy081 we should remove that listener as soon as a text buffer is disposed, and I plan to also "null" out some text buffer members on my side to avoid that forgetting to dispose a listener would cause such a leak in the future.

I am sorry for not noticing the logical leak back in October and thank you for bringing this issue back to my attention, such that I tried reproing again! ❤️

[cynecx]

@alexdima I really appreciate that you did take a look again. Great find! ❤️

However could you please take a look at my question above regarding the heap snapshot behavior? (tl;dr the heap usage shows 60MBish total heap size as compared to the actual process size when the large file is loaded. Shouldn’t the text buffer be on the heap?).

I just realized that the text buffer is stored as JSArrayBufferData, which would explain why it isn’t accounted in the total heap size. Am I right?

[alexdima]

@cynecx It is interesting, I don't think this is related to our usage of ArrayBuffers (which get tallied correctly by the Heap Snapshot).

But something is indeed off with the Heap Snapshot because the strings stored by the text buffer do appear in the heap snapshot, but they are counted with a very small size (28 bytes). I've also seen they are of type "ExternalOneByteString". These strings are being created from Buffers via a TextDecoder instance:

[sandy081]

First of all, sorry for causing this bug and added the fix. Here are the details about the bug and fix:

We have file based recommendations (FileBasedRecommendations ) which listen on text model additions to the editor and recommend extensions based on file extension & language. Recently, I added an improvement to this to check for recommendations when user changes the language of the file (more details here - #102823). For this, I listen on model's language change. I was disposing this listener only when FileBasedRecommendations class is disposed which caused this leak as it was holding the model even after the model got disposed.

Fixed it by disposing the model listeners the FileBasedRecommendations has, when model is getting disposed (onWillDispose). To confirm this here are the heap snapshots before and after the fix.

Before

After

Before: There is an increase in memory (~2.6MB) every time I open a large file and close it.
After: No hike in memory when I do the same.

[github-actions]

This bug has been fixed in to the latest release of VS Code Insiders!

@cynecx, you can help us out by confirming things are working as expected in the latest Insiders release. If things look good, please leave a comment with the text /verified to let us know. If not, please ensure you're on version ac165d7 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

[LittleStoney]

thanks,I'll use webstorm instead

[cynecx]

/verified

[shiduobin]

I had the same problem, and the memory explosion was high