Perf issues opening thousands of documents

[saurabhiam]

• VSCode Version: Code 1.17.0 (be377c0, 2017-10-04T23:33:45.303Z)
• OS Version: Linux x64 4.10.0-37-generic
• Extensions: the listing length exceeds browsers' URL characters limit

---

VSCode consuming high amount of CPU & memory on startup and becomes unresponsive.
I get "Window is not responding" message

Here's the CPU usage snapshot

Steps to Reproduce:

1. Open VSCode
2. Critical unresponsiveness is observed when vscode opens with an existing project with some files open.

Reproduces without extensions: No

[egamma]

@Saurabh-Sharma what kind of project (language, size) is this?

Since you mention that this cannot be reproduced without extensions, can you please help us to narrow down which extension is causing this: uninstall an extension, restart, observe whether problem persists. I understand that this painful since you have many extension installed. I would start with the extension that provides language services.

[saurabhiam]

@egamma I am using React over my project.
The issue was being caused by IntelliSense for CSS class names

On removing the extension, vscode worked fine.

Although the same is working fine with the version prior to VSCode Version: Code 1.17.0 (be377c0, 2017-10-04T23:33:45.303Z)

[egamma]

@Saurabh-Sharma thanks for following up. The fact that you say the extension wasn't a performance issue before 1.17 is a valuable hint that would help us to track the root cause of this issue.

update we believe that this is caused that in 1.17 the node_modules folder is no longer excluded when an extension is using the API vscode.workspace.findFiles (see next comment). To confirm this:

• is this an project that you can share and that we can profile? If not can you create a CPU profile for us (steps below).
• in your settings do you configure files.exclude or search.exclude?
• in your .gitignore file do you exclude node_modules?

If the workspace is not sharable can you do a CPU profile when the extension is installed. These are the Hit F1 > Toggle Developer Tools. In the overflow menu (see below) select 'More Tools > JavaScript Profiler'. In there select start, let it profile for 30 to 60 seconds, stop it, save it to a file and attach the file here. Since the problem happens on start-up reload the window after you started the profiler using F1>Reload Window.

[egamma]

The extension that triggers the CPU load is using:

vscode.workspace.findFiles(`**/*.{${languages}}`, '')` 

in 1.17 we have switched to use ripgrep. This has changed the defaults that are excluded from workspace.findFiles. When node_modules is not excluded in .gitignore then the node_modules folder is no longer excluded by default. I've confirmed this by using the extension with 1.16.

The proposed fix is:

• change findFile back to the old behaviour and to exclude node_modules by default. While this is not specified in the API, we should do this to avoid performance problems caused by extensions that rely on the old behaviour. There are several other extensions using this API function and they are affected by this (Docker, csharp, vscode-icons, ruby, typescript hero, ...)
• Make a PR against the CSS Intellisense extension.

[egamma]

The PR for the extension has been submitted zignd/HTML-CSS-Class-Completion#89

[egamma]

To reproduce the issue in VS Code.

• Install the HTML Hint, Markdown Lint extensions (they will report issues inside node modules)
• In VS Code 1.16, install the CSS Intellisense extension
• Git clone a repository with node modules with some markdown/html files. I've used https://github.com/Microsoft/vscode-npm-scripts.
• Open this workspace in VS Code
  Observe that the Problems View is empty.

Now do the same steps in VS Code 1.17. The Problems View will contain many errors, which shows that workspace.findFiles also returns files inside the node_modules folder. These files are then opened by the extension which triggers the linters, which then report errors against the files in node_modules.

[chrmarti]

I see the problem reported in 1.16.1 too:

[roblourens]

I think in #34434 and #34487 we discovered that node_modules and .git/ was never excluded, so I don't think that aspect of it is new. I still believe that we should apply files.exclude or just exclude those two by default.

Edit - I guess that was just for workspaceContains, never mind

[chrmarti]

findFiles() does apply files.exclude but not search.exclude which lists node_modules by default. My screenshot above the code indicate that node_modules wasn't excluded in 1.16.x.

@Saurabh-Sharma The CSS Intellisense extension just published a new version with the suggested fix. Could you check if you still see the problem with that newer version?

[egamma]

@chrmarti hmm, below is a screen shot from my setup and it shows that node_modules was excluded. My screenshot is from 1.16 and yours is from 1.16.1. So this means that 1.15 did exclude node_modules.

[chrmarti]

@egamma Did the extension auto-update in your install? Your change is already published in a new version. Do you have files.exclude set to something else than the default?

@jrieken When I open one of my large workspaces the 'Intellisense for CSS class names' extension quickly gets both 1.16 and 1.17 locked up. Spending a lot of time comparing URIs in: https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/api/electron-browser/mainThreadDocumentsAndEditors.ts#L107

Still looking for the regression.

[jrieken]

I'd say that's the effect of opening many, many documents. On every change we need to sync the documents and editor over. @chrmarti Do you have a real CPU profile that you can share?

[chrmarti]

@jrieken I do and I think you're right. I wonder if computing the diff with a set/map instead of sorted arrays would be faster. I've copied the profiles I currently have to //mt1/eng/35996, the one I'm currently looking at is the -master which is from unminified source.

@sandy081 Another hot spot in the profile is ConfigurationModel.override() that gets triggered from FileService.configuredAutoGuessEncoding(). That I haven't seen in the 1.16.1 profile, but I need to double-check.

I don't think we can explain the reported regression yet. 1.16 performs just as badly with that many opened documents as 1.17.

[chrmarti]

[diminutivesloop]

I'm seeing a similar issue, but it manifests even when I have no open projects and all extensions are disabled.

[chrmarti]

@diminutivesloop That looks like a different problem, could you open an new issue (Help > Report Issues)? Thanks.

[chrmarti]

@sandy081 ConfigurationModel.override() could clone the contents only when the first override that needs to be merged is found. For FileService.configuredAutoGuessEncoding() this could be optimized further because that only needs a single value, no need to compute the full config first.

The document sorting is the bigger issue though.

[bpasero]

@jrieken tried with your steps and those textFileEditorModel methods are not showing up in the top consuming methods for me, can you reproduce reliably and if so is there more to it than running #35996 (comment) on the vscode workspace?

My trace:
CPU-20171030T121210.cpuprofile.txt

Looking at your trace, the top consuming method seems to be this onStateChange handler which should be a no-op unless files are being reverted.

[jrieken]

Yes, always reproduces. @bpasero Did you really run this on our workspace with thousands of documents (4414 to be precise)? Your longest single method runs for just a 1sec, I have runtime in the area of 10-30seconds.

[bpasero]

@jrieken tried on vscode workspace and I am seeing ~1700 files returned but I can try with more files.

[bpasero]

@jrieken seems better to me after 40b58af can you check?

[jrieken]

Yeah, seems a lot better now, mostly garbage collecting (we should memory profile this) and object-clone (/cc @sandy081 on that)

after.cpuprofile.txt

[bpasero]

I also noticed the cloning appearing to the top.

[sandy081]

@bpasero @jrieken

Configuration is always cloned when is being asked, otherwise there could be a chance of changing the value in-memory which is error-prone. May I know where are the majority of the calls reading configuration are coming from ?

I see, in most of the places, we are asking the complete configuration even though we need value for a section or for a key. Making it specific, might reduce the cloning time.

[bpasero]

Looks like some calls are originating from file service finding out the configured encoding for each resource that gets loaded (here).

[jrieken]

Configuration is always cloned when is being asked, otherwise there could be a chance of changing the value in-memory which is error-prone.

@sandy081 Why not call object.freeze/deepfreeze once when creating those objects?

[sandy081]

@jrieken I have to do deepFreeze, but did not find a native deepFreeze method. If we do deepFreeze ourselves, won't be similar to deepClone?

[jrieken]

We used to have deepFreeze in objects.ts but we might have removed (unused). The idea is to walk the object recursively and freeze it in place (Object.freeze modifies the argument)

If we do deepFreeze ourselves, won't be similar to deepClone?

Well, you only do it once, when you compute/write the data, but not everytime you are asked for configuration (reading).

[vscodebot]

This issue has been closed automatically because it needs more information and has not had recent activity. Please refer to our guidelines for filing issues. Thank you for your contributions.

[sandy081]

Configuration service was made faster by not cloning always instead of freezing once and surfacing frozen object.