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

FSWatcher.close() incurs a big performance cost in >=10.16 #29949

Open
jonjaques opened this issue Oct 12, 2019 · 2 comments
Open

FSWatcher.close() incurs a big performance cost in >=10.16 #29949

jonjaques opened this issue Oct 12, 2019 · 2 comments
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@jonjaques
Copy link

  • Version: 10.16
  • Platform: Mac
  • Subsystem: fs

There seems to have been a massive performance decrease in FSWatcher.close() introduced in 10.16.

I use ts-node-dev as a file watcher when developing web applications, which under the hood uses filewatcher.

I believe the following commits/PRs could be contributing? Not smart enough to tell you why though 😅

Some examples w/ a pretty small express app:

10.15

$ nvm use 10.15
$ tsnd src/index 

save some files...

Using ts-node version 8.4.1, typescript version 3.6.4
[INFO] 17:03:16 Restarting: manual restart from user
removing 962 watchers
watcher.removeAll: 5.162ms
Using ts-node version 8.4.1, typescript version 3.6.4
[INFO] 17:03:17 Restarting: manual restart from user
removing 962 watchers
watcher.removeAll: 3.997ms
Using ts-node version 8.4.1, typescript version 3.6.4
[INFO] 17:03:18 Restarting: manual restart from user
removing 962 watchers
watcher.removeAll: 3.915ms
Using ts-node version 8.4.1, typescript version 3.6.4
[INFO] 17:03:18 Restarting: manual restart from user
removing 921 watchers
watcher.removeAll: 2.992ms

10.16

$ nvm use 10.16
$ tsnd src/index

save some files...

Using ts-node version 8.4.1, typescript version 3.6.4
[INFO] 17:05:20 Restarting: manual restart from user
removing 962 watchers
watcher.removeAll: 8631.891ms
Using ts-node version 8.4.1, typescript version 3.6.4
[INFO] 17:05:31 Restarting: manual restart from user
removing 962 watchers
watcher.removeAll: 8679.892ms
Using ts-node version 8.4.1, typescript version 3.6.4
[INFO] 17:05:42 Restarting: manual restart from user
removing 962 watchers
watcher.removeAll: 8679.248ms
Using ts-node version 8.4.1, typescript version 3.6.4
[INFO] 17:05:51 Restarting: manual restart from user
removing 962 watchers
watcher.removeAll: 8764.230ms
Using ts-node version 8.4.1, typescript version 3.6.4

Method

Perf code added to the following: https://github.com/fgnass/filewatcher/blob/master/index.js#L130

FileWatcher.prototype.removeAll = function() {
  console.time("watcher.removeAll");
  console.log(`removing ${this.list().length} watchers`);
  this.list().forEach(this.remove, this);
  console.timeEnd("watcher.removeAll");
};

One might be tempted to think the list() method could be slow, however adding code right above it, a single invocation of watcher.close() took longer than all of them in 10.15.

watcher.close ~/project/node_modules/date-fns/getISOWeekYear/index.js: 6.616ms
watcher.close ~/project/node_modules/date-fns/startOfISOWeek/index.js: 6.514ms
watcher.close ~/project/node_modules/date-fns/startOfWeek/index.js: 6.731ms
watcher.close ~/project/node_modules/date-fns/setISOWeekYear/index.js: 6.540ms
watcher.close ~/project/node_modules/date-fns/startOfISOWeekYear/index.js: 6.447ms
watcher.close ~/project/node_modules/date-fns/differenceInCalendarDays/index.js: 6.563ms
watcher.close ~/project/node_modules/date-fns/_lib/getTimezoneOffsetInMilliseconds/index.js: 6.465ms
@jonjaques jonjaques changed the title fs.FSWatcher.close() incurs a (large) performance decrease in >=10.16 FSWatcher.close() incurs a performance decrease in >=10.16 Oct 12, 2019
@jonjaques jonjaques changed the title FSWatcher.close() incurs a performance decrease in >=10.16 FSWatcher.close() incurs a big performance cost in >=10.16 Oct 12, 2019
@Fishrock123 Fishrock123 added the fs Issues and PRs related to the fs subsystem / file system. label Oct 14, 2019
@sebdvd
Copy link

sebdvd commented Sep 9, 2021

  • Version: 14.17.6
  • Platform: macos 11.5.2
  • Subsystem: fs

Hello, I noticed too that closing a lot of fs.FSWatcher instances were slow. I recently migrated some big projects to webpack@5 and webpack-dev-server@4, and I noticed that closing the dev server was slow (cf. discussion here webpack/webpack-dev-server#1479 (comment)). I dug in the source code and found that the webpack watch feature uses watchpack@2 and fs.watch and takes a lot of time to close all the watchers. In my case it was about closing ~1200 fs.FSWatcher. So I wrote a few lines of code to reproduce this issue:

const fs = require('fs');
const watchers = [];
for (let n = 0; n < 1200; n += 1) {
  watchers.push(fs.watch(process.cwd()));
}
console.log('closing watchers...');
console.time('closed');
for (let w of watchers) {
  w.close();
}
console.timeEnd('closed');

On my laptop it's quite slow, here the output:

➜ for i in {1..5}; do node script.js; done

closing watchers...
closed: 5.244s
closing watchers...
closed: 5.247s
closing watchers...
closed: 5.217s
closing watchers...
closed: 5.272s
closing watchers...
closed: 5.242s

@heyimalex
Copy link

Noticing a similar issue on macOS Monterey 12.4, intel, node: v16.16.0. Closing a watcher takes up to 50ms, so closing 1k+ in a tight loop leads to the process being busy for many seconds. Reproducing is trivial: create 5k watchers and then time the closes:

const fs = require("fs");
const { performance } = require("perf_hooks");

const filenames = [/* 3k+ paths to files/directories */]

filenames
  .map((filename) => fs.watch(filename))
  .forEach((watcher) => {
    const start = performance.now();
    watcher.close();
    const duration = performance.now() - start;
    console.log(duration);
  })

From the output of this it's pretty clear what's happening: the first durations printed are ~35ms and they gradually decrease to sub-ms times, so FSWatcher.close performance scales with the total number of watchers open. Here's close duration plotted against watchers open.

Screen Shot 2022-08-05 at 3 33 24 PM

Encountered this issue inside watchpack which "intelligently" replaces non-recursive watchers with recursive watchers when the total number of watchers hits some limit. When this replacement happens, watchpack will close the already-opened watchers, and if there are already a lot of files being watched this can take a long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

No branches or pull requests

4 participants