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

Mac: High CPU usage for large folders #447

Closed
bpasero opened this issue Mar 9, 2016 · 19 comments
Closed

Mac: High CPU usage for large folders #447

bpasero opened this issue Mar 9, 2016 · 19 comments
Labels

Comments

@bpasero
Copy link

bpasero commented Mar 9, 2016

This is more a question than a bug report with concrete steps. VS Code (using chokidar 1.0.5) has numerous bug reports where Mac users report high CPU usage for our file watcher. The only thing in common is that these users open VS Code on a very large folder (> 2 GB) which seems to cause the high CPU spikes.

Is there any known issue for chokidar or fsevents where on a Mac CPU would go crazy when a folder is large? I would assume that the folder size has no impact on how chokidar/fsevents behaves but maybe there is something I am missing?

@es128
Copy link
Contributor

es128 commented Mar 9, 2016

The physical size of the files shouldn't have an impact, but a deeply nested directory tree with many entries would have an impact on the initial scan of the file structure. It should subside after the ready event if the paths were passed with a new watcher instance (if using .add() the process is the same, but there is no similar ready signal when the scan is done in that case).

Also, using usePolling: true in such a situation would cause persistently high CPU utilization.

@bpasero
Copy link
Author

bpasero commented Mar 9, 2016

@es128 interesting, I was not aware of the initial scan, is there a reason why you need to scan all files on startup? does this scan ignore folders that are excluded with the exclude setting?

@es128
Copy link
Contributor

es128 commented Mar 9, 2016

Skimmed through the linked issue. Switching versions of node with nvm will break fsevents and cause chokidar to silently revert to polling. The solution is to npm rebuild after any change in the version of node. This presumes that VS Code uses whichever version of node is in $PATH as opposed to locking down to a bundled version.

@bpasero
Copy link
Author

bpasero commented Mar 9, 2016

@es128 I think this issue is independent and only shows up when running Code from its source code form. When running the bundled product, the environment is made of the node environment Electron provides and is always stable independent from the node version installed.

@es128
Copy link
Contributor

es128 commented Mar 9, 2016

You and I have discussed the initial scan with readdirp in the past. Chokidar needs a reference point for the file tree in order to correct erratic events that come from the lower-level watch methods, such as signals emitted on a directory when a file was added or removed. It does avoid recursing through ignored paths.

The paths it is keeping track of can now be exposed with the .getWatched() method.

Just FYI - there's been some movement toward making the scan more efficient (#412), but I'll need to focus more time and attention toward it at some point to work toward landing those improvements.

@es128
Copy link
Contributor

es128 commented Mar 9, 2016

But I don't think the scan has much to do with the original issue report. The symptoms described in this comment make it pretty clear to me that for whatever reason polling mode is being used in that case.

@bpasero
Copy link
Author

bpasero commented Mar 9, 2016

@es128 yes that was my theory as well but microsoft/vscode#3222 (comment) made me think otherwise:

However, that said, there appears to be no difference in CPU usage either way. Even with fsevents loaded properly Activity Monitor still reports Electron Helper at 108% CPU and my laptop is about to fly off my desk...

@es128
Copy link
Contributor

es128 commented Mar 9, 2016

Ah, yes sorry, I was focusing on the comment about how CPU usage stays up after 30 minutes, but in the prior paragraph the description does point more toward the initial recursive scan of the file tree.

But you've had trouble reproducing with a similarly large file tree on a Mac? Not sure how to explain the discrepancy. I suppose hardware differences could have an impact.

If you have a way to ignore node_modules, at least until the user chooses explicitly to explore it, that should help tremendously.

@bpasero
Copy link
Author

bpasero commented Mar 9, 2016

@es128 well I have to take that back, when I now open a folder with the chrome sources I see the CPU go up for maybe 10 seconds. I realize this is the initial file scan that you guys do but afterwards the CPU stays low.

I do think about introducing a way to exclude large folders in VS Code for file watching, but it feels like a workaround for something that ideally gets fixed on the watcher layer. I find #412 a good start to see how to speed up this initial scan. Unfortunately my own experiments have shown that a disk scan is always eating CPU cycles simply because modern disks are so fast that they keep you busy for a while...

@bpasero
Copy link
Author

bpasero commented Mar 11, 2016

@es128 we found out about the issue and it seems #412 would fix it. here is what happens:

  • user opens a large folder in VS Code
  • VS Code installs a watcher on the root of the folder from another process
  • chokidar starts to walk the folder in a way that memory increases a lot
  • eventually the process dies with out of memory
  • VS Code detects this and starts the watcher again because we typically never want to loose the watching information
  • this goes on and on forever and simply results in constant and high CPU use

We also verified that chokidar was not falling back to polling.

My current fix is:

  • allow the user to configure excludes
  • do not restart chokidar after 5 failing attempts

I am wondering if VS Code should configure chokidar differently if it detects that it fails for N times. For example, we could put chokidar into polling mode with a very long polling interval. Or do you have other ideas?

@es128
Copy link
Contributor

es128 commented Mar 11, 2016

Polling will only make it worse, the readdirp scan process stays the same, and then you eat even more CPU by installing all those polling watchers, even if they are at long intervals. Looks like we really need to drive #412 home. I would appreciate help with that.

Have you actually used it in the same situation where you were able to reproduce the problem and seen that it eliminated it? Any ideas what the relative limits are of the new file walker? Is it possible to add enough files that it falters too?

Do you have any concerns related to your feedback that you'd want addressed before we move toward making this change?

@bpasero
Copy link
Author

bpasero commented Mar 11, 2016

Thanks, I also think polling is the worst of all solutions.

I did not test chokidar with the suggested changes yet but can dedicate some time after our GA (end of march) to look into this. One issue I am already seeing is that chokidar grew in size from previously 1.5 MB (1.0.5) to 5.5 MB (1.4.3) which might block us from advancing because we are sensitive to download size.

I think my feedback is valid for the case of protecting against endless loops due to cyclic links in the file system. I did some benchmarks with fs.readdir vs. fs.lstat and found that eager fs.readdir calls are not good. However I think this feedback is more for readdirp-filtered than chokidar. I like how simple the change for chokidar will be because it is just a drop in replacement without much changes.

@es128
Copy link
Contributor

es128 commented Mar 11, 2016

Regarding size, perhaps we should resume the discussion at fsevents/fsevents#93 (comment). Don't want to derail this issue with it though.

@bpasero
Copy link
Author

bpasero commented Mar 11, 2016

Makes sense.

@bumpmann
Copy link

This is probably because fsevents is not loaded and the node watchers are really slow.

you can check that it's loaded:

let watcher = chokidar.watch(...)

if (process.platform === 'darwin' && ! watcher.options.useFsEvents) {
    console.error('Watcher is not using native fsevents library and is falling back to unefficient polling.');
}

and fix it with compiling it for electron: electron.atom.io/docs/tutorial/using-native-node-modules/

@woutersamaey
Copy link

woutersamaey commented Jul 15, 2019

@bumpmann watcher.options is undefined. Any ideas how to get it? Love your idea of showing a warning message!! For now I've settled on using the FsEventsHandler.canUse() method to do the same thing, but it would be a little better to get it from Chokidar directly.

@bpasero
Copy link
Author

bpasero commented Jul 15, 2019

This is NOT because fsevents is not getting loaded btw. I tried using node.js standalone, no Electron involved. See #860

@woutersamaey
Copy link

@bpasero in my case a simple npm rebuild fixed my issue, but your case may be different. Having a warning so we know when npm rebuild is needed is a lifesaver.

@paulmillr
Copy link
Owner

I think this can be closed, the most recent versions should've improved this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants