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

Make watchgod 10-100x faster #75

Merged
merged 1 commit into from
Jan 30, 2021
Merged

Conversation

fabioz
Copy link
Contributor

@fabioz fabioz commented Jan 30, 2021

I was considering watchgod for doing some file-watching and while reviewing the code I found one issue that I think is critical: watchgod is not taking full advantage of the scandir result!

Instead of doing a new os.stat for each file it should reuse what scandir already did (saving thousands of stat calls depending on the scenario).

I have some different targets, so, I won't be really use watchgod, but as I think this is really critical I decided to provide a pull request ;)

@fabioz fabioz changed the title Make watchdog 10-100x faster Make watchgog 10-100x faster Jan 30, 2021
@fabioz fabioz changed the title Make watchgog 10-100x faster Make watchgod 10-100x faster Jan 30, 2021
@fabioz
Copy link
Contributor Author

fabioz commented Jan 30, 2021

As a note, it seems I have a typo in the commit (watchdog instead of watchgod), please rebase the fix to provide a proper commit message ;)

@samuelcolvin
Copy link
Owner

Thanks, this is a really good catch. I thought I looked at this when first creating the library, but either the docs/functionality have changed or I completely missed this.

Long term I want to switch to something like #25, but this is definitely a worth while fix until I get around to that.

As a note, it seems I have a typo in the commit (watchdog instead of watchgod), please rebase the fix to provide a proper commit message ;)

No problem, as noted in the readme my choice of name was definitely a mistake.

@codecov
Copy link

codecov bot commented Jan 30, 2021

Codecov Report

Merging #75 (a9051a5) into master (26a1219) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #75   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          280       280           
  Branches        29        29           
=========================================
  Hits           280       280           

@samuelcolvin samuelcolvin merged commit 6b30208 into samuelcolvin:master Jan 30, 2021
@samuelcolvin
Copy link
Owner

thanks so much, I'll run through other PRs and make a new release today.

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