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

fix(watcher): use exclude options to optimize file watching #1320

Merged
merged 3 commits into from
Nov 8, 2019

Conversation

edvald
Copy link
Collaborator

@edvald edvald commented Nov 6, 2019

What this PR does / why we need it:

This basically relies on optimizations already implemented in chokidar,
the library we use for file watching, in order to reduce overhead when using file watching on linux. Hopefully this can solve the
excessive CPU/RAM usage some users have reported.

Which issue(s) this PR fixes:

Closes #1269

Special notes for your reviewer:

In a large project folder, try adding exclusions via modules.exclude in the project config, or using the exclude field in individual module configs.

There are some potential alternative approaches, but I wanted to see if this works reasonably before doing something more elaborate.

@edvald
Copy link
Collaborator Author

edvald commented Nov 6, 2019

@matuszeman could you try this and see if it solves your issue?

@eysi09
Copy link
Collaborator

eysi09 commented Nov 6, 2019

It's a little confusing that exclude changes the watch behaviour whereas include does not. Perhaps we can mention this in the reference docs as well. For both the include and the exclude directive for that matter, so it's clear that they behave in subtly different ways.

@edvald
Copy link
Collaborator Author

edvald commented Nov 6, 2019

I did put in clauses in the docs mentioning that these specific fields can be used to limit file watching, but didn't explain why the include filters aren't helpful.

The reason is that only rarely/incidentally can include statements help us optimize anything. Basically the only scenario where we could in theory use them to limit the scope, would be when all of the include filters are directory filters (e.g. my/directory/**/* or my/directory/*). Otherwise we need to match anything matching the filter, which can then be in any directory, and chokidar has no optimizations for that scenario.

I'm not sure where/how to explain this though. What do you think?

@eysi09
Copy link
Collaborator

eysi09 commented Nov 6, 2019

Yeah I agree with the implementation. I suggested mentioning this in the reference docs for the include and exclude directives.

I figured users would expect include to also affect the watch behaviour since that's what exclude does. So explicitly mentioning that it doesn't in the include reference docs might prevent confusion.

@edvald
Copy link
Collaborator Author

edvald commented Nov 6, 2019

Sure, makes sense! I'll add notes in the relevant schema field descriptions.

This basically relies on optimizations already implemented in chokidar,
the library we use for file watching. Hopefully this can solve the
excessive CPU/RAM usage some users have reported.

Closes #1269
@edvald
Copy link
Collaborator Author

edvald commented Nov 7, 2019

I've added a commit, and done some more testing. I believe this is good to go now.

Note that I've removed the optimization based on module-level exclude fields for now. Turns out using module exclude fields would need more elaborate logic, in the case of overlapping module sources.

@edvald edvald changed the title fix(watcher): use exclude options to optimize file watching (WIP) fix(watcher): use exclude options to optimize file watching Nov 7, 2019
@edvald edvald requested a review from eysi09 November 7, 2019 19:11
Copy link
Collaborator

@eysi09 eysi09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@edvald edvald force-pushed the watch-excludes branch 5 times, most recently from 6d30b7c to e2a64ae Compare November 8, 2019 12:51
Turns out using module exclude fields would need more elaborate logic,
in the case of overlapping module sources. So I removed that
optimization for now.
@edvald edvald merged commit aa82e89 into master Nov 8, 2019
@edvald edvald deleted the watch-excludes branch November 8, 2019 14:34
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.

File watching optimization
2 participants