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

Better: change watcher to properly filter ignored directories #1597

Merged
merged 6 commits into from
Aug 21, 2024

Conversation

ylecuyer
Copy link
Contributor

Lately I started to work on a node project with a lot of dependencies, when opening this project in ungit I started to get this error:

 2024-08-18T18:21:39.758Z - error: Error: ENOSPC: System limit for number of file watchers reached, watch '/home/ylecuyer/Projects/project/.git/index.lock'

So far I was able to get ungit running by changing the limit of inotify nodes doing

sudo sysctl fs.inotify.max_user_watches=524288 && sudo sysctl -p

And when checking the number of used inotify nodes, It is really big (263k):

ylecuyer@inwin:~/Projects/inotify-info$ ./_release/inotify-info 
------------------------------------------------------------------------------
INotify Limits:
  max_queued_events    16,384
  max_user_instances   128
  max_user_watches     524,288
------------------------------------------------------------------------------
       Pid Uid        App                     Watches  Instances
     14596 1000       node                    263,061          1

After investigating, I noticed that since node 20, native support for recursive has been added to fs.watch and the current watcher library used in ungit uses the native watcher when available (https://github.com/yuanchuan/node-watch/blob/main/lib/has-native-recursive.js#L6). The problem is that the filtering (to remove .gitignore files and node_modules files) is done only when the non native version is used.

I couldn't find a way to improve the current library to get it to work with native, so I just changed it for another one, there are a lot so I took one of them without benchmarking.

With the new watcher library the number of inotify nodes for the same project is now normal:

ylecuyer@inwin:~/Projects/inotify-info$ ./_release/inotify-info 
------------------------------------------------------------------------------
INotify Limits:
  max_queued_events    16,384
  max_user_instances   128
  max_user_watches     524,288
------------------------------------------------------------------------------
       Pid Uid        App                     Watches  Instances
     15510 1000       node                     13,494          1

@ylecuyer ylecuyer changed the title Change watcher yle Better: change watcher to properly filter ignored directories Aug 19, 2024
@campersau
Copy link
Collaborator

Looks good, lets try this one out, thanks!

@campersau campersau merged commit b2c1a04 into FredrikNoren:master Aug 21, 2024
10 checks passed
@ylecuyer ylecuyer deleted the change-watcher-yle branch August 21, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants