Skip to content

✨ Optimize batch path watching via Watcher::paths_mut - #53

Merged
roma-glushko merged 2 commits into
roma-glushko:mainfrom
c-h-russell-walker:use_paths_mut_for_bulk_watching
Mar 9, 2026
Merged

roma-glushko merged 2 commits into
roma-glushko:mainfrom
c-h-russell-walker:use_paths_mut_for_bulk_watching

Conversation

@c-h-russell-walker

Copy link
Copy Markdown
Contributor

RATIONALE

This was a recent update that I believe this library could benefit from leveraging.

Here is a comment from a user (comment on same PR that is linked below) expressing expectation that this will solve some performance issues they were dealing with when there were many files being watched:
notify-rs/notify#692 (comment)


@roma-glushko curious to hear your feedback on this PR - thanks so much. for the support on this library.

RELATED

PR introducing Watcher::paths_mut
notify-rs/notify#692


Docs for Watcher::paths_mut:
https://docs.rs/notify/latest/notify/trait.Watcher.html#method.paths_mut

@roma-glushko
roma-glushko requested a review from Copilot March 9, 2026 16:48
@roma-glushko roma-glushko added the enhancement New feature or request label Mar 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Watcher::watch method to use the new notify v8 paths_mut() / batched-add API instead of calling self.inner.watch() individually per path. The motivation is a performance improvement when registering many paths at once, as described in notify PR #692.

Changes:

  • Acquire a WatcherPaths guard via self.inner.paths_mut() before the loop and batch all add calls within it.
  • Replace the per-path self.inner.watch(&path, mode) call with watcher_paths.add(Path::new(&p), mode).
  • Call watcher_paths.commit().ok() after the loop to apply all staged paths.
Comments suppressed due to low confidence (1)

src/watcher.rs:108

  • When an early return Err(...) is triggered inside the loop — either due to a non-existent path (line 97–100) or a non-permission-error watcher error (line 107) — watcher_paths.commit() on line 114 is never reached.

According to the notify docs for WatcherPaths, calling add without a subsequent commit means the changes are staged but never applied. Any paths that were successfully queued via watcher_paths.add(...) before the early return will be silently discarded without ever being watched. This is a behavioral regression compared to the old self.inner.watch(&path, mode) loop, where previously-successful iterations were applied immediately and independently.

To fix this, consider calling watcher_paths.commit() (and propagating the error) before each early return, or restructuring the loop so that validation is done before any paths_mut / add calls are made.

            if !path.exists() {
                return Err(PyFileNotFoundError::new_err(format!(
                    "No such file or directory: {}",
                    p
                )));
            }

            let result = watcher_paths.add(Path::new(&p), mode);

            if let Err(err) = result {
                if !ignore_perm {
                    return Err(map_notify_error(err));
                }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/watcher.rs Outdated

@roma-glushko roma-glushko left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for the improvement! Looks very useful.

@roma-glushko roma-glushko changed the title [CRW] Use Watcher::paths_mut for adding many paths at once ✨ Optimize batch path watching via Watcher::paths_mut Mar 9, 2026
@roma-glushko
roma-glushko merged commit 17c09f4 into roma-glushko:main Mar 9, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants