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

Replace watcher.js with broccoli-sane-watcher. #163

Conversation

jschilli
Copy link

@jschilli jschilli commented Jun 6, 2014

Replaces watcher.js with @krisselden's broccoli-sane-watcher

Should resolve #72 and replace #150

On our largish ember.js project we went from consistent 25% cpu with current watcher to roughly 0% using the sane-watcher

cc: @stefanpenner @rondale-sc @krisselden

@stefanpenner
Copy link
Contributor

nice!

@stefanpenner
Copy link
Contributor

I think... there might be a bug in the watcher.
broccolijs/broccoli-sane-watcher#3

We should confirm, if it is the case I imagine it wont be hard to fix.

@stefanpenner
Copy link
Contributor

broccoli-sane-watcher has been updated.

@amasad
Copy link

amasad commented Jun 10, 2014

As another data point of this vs Chokidar:

  1. Chokidar depends on native modules (annoying at the least and makes it unusable in some settings were you can't install native modules)
  2. I tried to use Chokidar at Facebook and was impossibly slow to startup and had a delay on change events when watching a lot of files.
  3. Eats up a lot more resources than sane because creates a lot more watcher instances
  4. Not sure if you care about that here but chokidar has no ready event so if you have a big project and it's crawling, creating instances, it's impossible to know when it's done

@krisselden
Copy link
Contributor

That's a TODO, I'm not using the ready event yet, either outputing to the user that it is now in fact watching, or double checking that the tree is still the same as when it was added.

@krisselden
Copy link
Contributor

I had planned broccoli-sane-watcher to become a PR for broccoli, not for broccoli to depend on it. Right now that is circular, since broccoli-sane-watcher depends on broccoli for printSlowTrees. If we go this route, printSlowTrees should be a module, and watching should be more pluggable with broccoli. /cc @joliss @stefanpenner

@stefanpenner
Copy link
Contributor

@krisselden i'll extract the logging for you -> broccolijs/broccoli-sane-watcher#5 (assigned myself)

@krisselden
Copy link
Contributor

@joliss should this be closed? You have a different plan for integration of broccoli-sane-watcher, confirm?

@stefanpenner
Copy link
Contributor

@krisselden ya i believe she does.

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.

Refactor Watcher to not poll?
5 participants