only use recursive watchers on macOS and windows #4100
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fallback to non-recursively watch read directories on other OS
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
8 Ignored Deployments
|
|
Benchmark for e2a9b99
Click to view full benchmark
|
Benchmark for 01255ab
Click to view full benchmark
|
kwonoj
approved these changes
Mar 7, 2023
sokra
added
the
pr: automerge
jridgewell
reviewed
Mar 7, 2023
| @@ -29,7 +29,7 @@ use std::{ | |||
| path::{Path, PathBuf, MAIN_SEPARATOR}, | |||
| sync::{ | |||
| mpsc::{channel, RecvError, TryRecvError}, | |||
| Arc, Mutex, | |||
| Arc, Mutex, MutexGuard, | |||
There was a problem hiding this comment.
MutexGuard is unused on Mac/Windows, giving a clippy warning.
| #[cfg(not(any(target_os = "macos", target_os = "windows")))] | ||
| fn restore_if_watching(&self, dir_path: &Path, root_path: &Path) -> Result<()> { | ||
| if self.watching.contains(dir_path) { | ||
| let mut watcher = self.watcher.lock().unwrap(); |
There was a problem hiding this comment.
We have a lock that's only used inside a loop. If we passed the HashSet to this method, we could lock once and iterate
Comment on lines
+229
to
+230
| let disk_watcher = self.watcher.clone(); | ||
| let root_path = self.root_path().to_path_buf(); |
There was a problem hiding this comment.
Ditto unused. Could we make a consistent interface between Mac and Linux so we can just make calls to empty methods?
| #[cfg(not(any(target_os = "macos", target_os = "windows")))] | ||
| batched_new_paths.insert(path.clone()); | ||
| } | ||
| Ok(DebouncedEvent::Remove(path)) => { |
There was a problem hiding this comment.
Do we need to remove a from the DiskWatcher in this case?
jridgewell
added a commit
that referenced
this pull request
Mar 7, 2023
This reverts commit c0bb681.
mehulkar
pushed a commit
that referenced
this pull request
Mar 7, 2023
### Description Only windows and macOS support real recursive file watchers, other OS emulate it by walking the directory structure and watching all directories. But that might be really slow and use up a lot of watchers, so we don't want that. Instead we know exactly which directories are used and can watch selectively directories when they are accessed. * only use recursive watchers on macOS and windows * fallback to non-recursively watch read directories on other OS Note that this implementation still has some bugs when renaming folders, but we can probably figure out these edge cases later...
kodiakhq bot
pushed a commit
to vercel/next.js
that referenced
this pull request
Mar 8, 2023
## Bugfixes - vercel/turborepo#3526 - vercel/turborepo#4084 - vercel/turborepo#4083 - vercel/turborepo#4067 - vercel/turborepo#3959 - vercel/turborepo#4060 - vercel/turborepo#4081 - vercel/turborepo#4103 - vercel/turborepo#4100 - vercel/turborepo#4108 - vercel/turborepo#4101 - vercel/turborepo#4112 - vercel/turborepo#3956 - vercel/turborepo#4118 - vercel/turborepo#4117 ## Feature - vercel/turborepo#4058 - vercel/turborepo#4013 - vercel/turborepo#4005 - vercel/turborepo#4001 - vercel/turborepo#3839 - vercel/turborepo#4017 - vercel/turborepo#4086 - vercel/turborepo#4067 ## Performance - vercel/turborepo#4040 - vercel/turborepo#4110 ## Refactor - vercel/turborepo#4098
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Jul 25, 2024
### Description Only windows and macOS support real recursive file watchers, other OS emulate it by walking the directory structure and watching all directories. But that might be really slow and use up a lot of watchers, so we don't want that. Instead we know exactly which directories are used and can watch selectively directories when they are accessed. * only use recursive watchers on macOS and windows * fallback to non-recursively watch read directories on other OS Note that this implementation still has some bugs when renaming folders, but we can probably figure out these edge cases later...
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Jul 29, 2024
### Description Only windows and macOS support real recursive file watchers, other OS emulate it by walking the directory structure and watching all directories. But that might be really slow and use up a lot of watchers, so we don't want that. Instead we know exactly which directories are used and can watch selectively directories when they are accessed. * only use recursive watchers on macOS and windows * fallback to non-recursively watch read directories on other OS Note that this implementation still has some bugs when renaming folders, but we can probably figure out these edge cases later...
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Aug 1, 2024
### Description Only windows and macOS support real recursive file watchers, other OS emulate it by walking the directory structure and watching all directories. But that might be really slow and use up a lot of watchers, so we don't want that. Instead we know exactly which directories are used and can watch selectively directories when they are accessed. * only use recursive watchers on macOS and windows * fallback to non-recursively watch read directories on other OS Note that this implementation still has some bugs when renaming folders, but we can probably figure out these edge cases later...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Only windows and macOS support real recursive file watchers, other OS emulate it by walking the directory structure and watching all directories. But that might be really slow and use up a lot of watchers, so we don't want that. Instead we know exactly which directories are used and can watch selectively directories when they are accessed.
Note that this implementation still has some bugs when renaming folders, but we can probably figure out these edge cases later...