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

No way to listen to a directory non-recursively #111

Closed
nex3 opened this issue May 10, 2013 · 18 comments
Closed

No way to listen to a directory non-recursively #111

nex3 opened this issue May 10, 2013 · 18 comments
Assignees
Labels
✨ Feature Adds a new feature
Milestone

Comments

@nex3
Copy link
Contributor

nex3 commented May 10, 2013

As of 1.0.0, when listening to a directory, all subdirectories of that directory are also automatically listened to. Sometimes this isn't the desired behavior, but there's no (obvious) way around it other than manually ignoring all changes in subdirectories.

@thibaudgg
Copy link
Member

Good point, we could add an option like scan_deep_level (or any better name...) that could affect all directories watched.
Do you think it would be enough?

@nex3
Copy link
Contributor Author

nex3 commented May 13, 2013

Yeah, that would be sufficient.

@thibaudgg
Copy link
Member

Great, I'll work on it once the Listen rewrite will be done.

@nex3
Copy link
Contributor Author

nex3 commented Sep 12, 2013

According to @stephanvane's comment on sass/sass#764, this is fixed in listen 1.3.1. Is that accurate?

@thibaudgg
Copy link
Member

It is not, I'm still working on the rewrite for the v2.0 version. That feature should be added in v2.x

@stephanvane
Copy link

Sorry about that, guess I misread some comments :(. Good luck on the rewrite!

@thibaudgg
Copy link
Member

@stephanvane rewrite is mostly done now, feel free to give a try to the v2.0 branch. I should release a pre-release in the next few days. Thanks!

@thibaudgg
Copy link
Member

Listen 2.0.0 has just been released, so I would have some time to implement this feature.

Is it still needed? Isn't it something that could be done with the ignore option via a simple regex? Or maybe with a new option like Listen.to 'dir', only: /some sub dir/?

@nex3
Copy link
Contributor Author

nex3 commented Oct 9, 2013

The goal here is to make watching individual files as efficient as possible. Right now they're O(n) for the size of the entire recursive contents of the directory containing them, which is very bad. If we could disable recursive watching, that could at least make them O(n) for the number of sibling files, which would be a marked improvement. It sounds like ignore filters files after they're returned from the underlying watcher, which means that it doesn't solve the issue.

@thibaudgg
Copy link
Member

Performance really depends on which adapter is used. Polling adapter will really benefit of an ignore/only filter because ignored dir/file will not be accessed, on specific system adapters it's more complicated as each one have a different way of handling listening (and the majority of them doesn't allow ignoring patterns). But even if the ignore is skipped at the adapter level it'll still be useful and will improve Listen performance.

I would prefer to not add new options that only apply to a specific adapter (i.e.: only rb-inotify adapter).
By curiosity, is it possible to avoid recursivity scanning with rb-inotify gem?

@nex3
Copy link
Contributor Author

nex3 commented Oct 10, 2013

Being able to listen non-recursively or listen to a single file is generally useful. A library like listen is most helpful to me as a user when it exposes functionality that's useful and takes care of implementing it in terms of underlying primitives itself.

That said, yes, rb-inotify supports non-recursive scanning. In fact, inotify itself only supports non-recursive scanning, and rb-inotify builds its recursive scanning interface on top of this by automatically listening and un-listening to new directories.

Since non-recursive scanning is useful and can be implemented efficiently with at least the inotify and polling adapters, it seems like a clear case for new functionality.

@thibaudgg
Copy link
Member

Ok good arguments, I agree that we can add this option even if it'll more effective on polling and inotify.

On rb-inotify I only the the :recursive option, so we can't set the level right? If true, I'll just add the simple recursive: true/false option to Listen.

Thanks for your feedback!

@whitehat101
Copy link

👍 I just thought of a use for this feature. Set-up some configuration when a dir is created, and takedown that config when it is removed. I'll directly use rb-inotify for this project.

@e2
Copy link
Contributor

e2 commented May 22, 2014

TL;DR; - will likely be implemented soon once it's worked out for every platform

This is a little tough to do, since every adapter does things differently:

  1. rb-inotify - implements it's own recursion (no problem here, can be extracted to Listen)
  2. wmd - natively does recursion (though can reuse implementation from 1 to be better supported)
  3. fsevent - natively does recursion, and I have no idea if this can be (or should be) disabled or not (ideally, it would reuse 1, event at the slight performance hit)
  4. bsd - currently probably broken (but basically implemented recursion using Find, so same as 1)

@thibaudgg - any advice about rb-fsevent and recursion? Can it be turned off? Is it worth it? Would watching dirs (and new ones) explicitly and without recursion be a big performance hit?

@thibaudgg
Copy link
Member

@e2 I don't think it can be turned off and I don't think it makes more sense to doing so. It should be fine like that.

@e2
Copy link
Contributor

e2 commented May 22, 2014

Thanks for the answer, @thibaudgg - much appreciated.

Crap. So my idea of making recursion option os-independent goes out the window.

Implementing recursion: false means ignoring files for fsevent, which means recursion stays os-specific, which means directory config separation stays os-specific, which means symlink handling has to be os-specific.

I'll work something out...

@e2
Copy link
Contributor

e2 commented Jun 5, 2014

TL;DR the actual work required to support this instead of using a regexp outweighs the costs in any use case I can think of

@nex3 - I'm closing this, because:

  • v2.7.7 (just released) contains a massive speed improvement for large directories (indexing hundreds of thousands of files recursively now takes just seconds on Linux)
  • the WDM adapter handles recursion internally well enough (listening to single directories would provide little benefit and I don't know of a case where recursion is the bottleneck)
  • the rb-fsevent gem supports only recursion - it always returns events in bulk, so non-recursion would only make it much, much slower, while any performance "benefit" would be eliminated just by the rb-fsevent's lag time
  • rb-inotify is really the only adapter which is configurable for this, and even so it's a TON of work to properly implement the option and propagate it throughout the current Listen architecture (which is HEAVILY refactored towards recursion, while adding the option would MULTIPLY test cases)
  • polling shouldn't be used for large projects anyway (even though the v2.7.7 fix helps with the indexing problem), while it's based on ignores anyway and File::stat calls with recursion is not the bottleneck on a cached filesystem, and there's no memory overhead either.
  • supporting recursion: false only in Linux doesn't make sense for a cross-platform library like Listen, while implementing AND TESTING it on ALL the other platforms is more work than anyone is currently willing to take on (it seems trivial, but it's lots precious time spent, giving no worthwhile or noticeable improvements)
  • given that this is platform-specific functionality and there are no cross-platform tests for all adapters (I took a stab at this, but it's too much work), the only likely "result" is breaking functionality in the remaining adapters - making Listen useless on anything except Linux, which would make it almost no different than using rb-inotify directly.

And the workarounds are trivial:

  • using the :only like @thibaudgg mentioned should fix the problem (any "non-recursion" "non-inotify" adapter would basically be doing this, anyway)
  • using symbolic links should help work around any performance problem, by physically moving watched files into a separate directory, and linking to them from the "big" directory
  • the upcoming new Silencer has an API that easily allows defining complex ignore/accept rules per directory watched (so there's no reason to provide a generic recursion: false option in Listen to handle a specific/rare use case in a given project)
  • using an SSD drive, fs-optimizations, disabling atime, more RAM - those should solve the problem for professionals, while beginners have a polling warning.
  • rb-inotify is in Ruby and has a similarly easy-to-use API, so forking projects and changing them to use rb-inotify (instead of Listen) is trivial.
  • in the new API, I'm planning on allowing to configure adapters with their own options - but with no support for them (e.g. you could redefine which events rb-inotify listens to, etc.) - but I don't intend on personally supporting non-recursion (except for accepting pull requests).

If I'm wrong, and there's a real-life use case where above workarounds -- especially the ignore rules or symlinking won't do -- and where spending lots of hours to put this into Listen makes sense ... feel free to reopen this.

And of course, pull requests are more than welcome ;)

@e2 e2 closed this as completed Jun 5, 2014
@e2
Copy link
Contributor

e2 commented Nov 15, 2014

I've redefined this problem as not being able to prevent listen from scanning ignored directories: #274

(Users shouldn't care about recursion - they should care only about setting up proper ignore rules).

y-yagi added a commit to y-yagi/rails that referenced this issue Jun 2, 2019
`spring-watcher-listen` watch application root by default.
https://github.com/jonleighton/spring-watcher-listen/blob/c4bfe15805867e229588e4b776a7b87438137094/lib/spring/watcher/listen.rb#L58

This is necessary to watch the file (e.g. `.ruby-version`) in the
application root.

By this `node_modules` also be watched, and it is a possibility to be
shown a warning by `listen`.
Related to rails#32700, rails#34912, rails/webpacker#1990.

`listen` watches directory recursive by default, and it cannot avoid it.
guard/listen#111

So If this warning happens, the only workaround the user can do is remove
the gem.

The issue is likely to occur more frequently in Rails 6 because
`rails new` runs `webpacker:install` by default. Because of such a
state, I think that we should not recommend to use
`spring-watcher-listen`.

Spring has polling watcher, restart process works without this
`spring-watcher-listen`.
Because of polling base, CPU load may be higher than listen base. Still
I think that it is better than the warning comes out.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature Adds a new feature
Projects
None yet
Development

No branches or pull requests

5 participants