refactor(turborepo): Improve package changes watcher performance #8064
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
5 Ignored Deployments
|
NicholasLYang
force-pushed
the
nicholasyang/package-change-watcher-perf
branch
from
64cd188
to
09792fa
Compare
🟢 Turbopack Benchmark CI successful 🟢Thanks |
NicholasLYang
force-pushed
the
nicholasyang/package-change-watcher-perf
branch
from
09792fa
to
adf977b
Compare
|
|
||
| let changed_packages = change_mapper.changed_packages(changed_files.clone(), None); | ||
|
|
||
| tracing::warn!("changed_files: {:?}", changed_files); |
There was a problem hiding this comment.
warn is probably not the right level semantically, but debug has so much stuff in it and we don't log it out by default
| changed_pkgs | ||
| ); | ||
| } | ||
| interval.tick().await; |
There was a problem hiding this comment.
100ms should be enough time that we don't need to park/notify, right?
There was a problem hiding this comment.
Yeah, I think that should be ok. We can ITG once the feature is stable.
NicholasLYang
force-pushed
the
nicholasyang/package-change-watcher-perf
branch
from
d22934b
to
5aba94a
Compare
🟢 CI successful 🟢Thanks |
| changed_pkgs | ||
| ); | ||
| } | ||
| interval.tick().await; |
There was a problem hiding this comment.
Yeah, I think that should be ok. We can ITG once the feature is stable.
| let changed_files: HashSet<_> = trie | ||
| .keys() | ||
| .filter_map(|p| { | ||
| let p = AbsoluteSystemPathBuf::try_from(p.as_path()).ok()?; |
There was a problem hiding this comment.
Should this be .expect("filewatching returns absolute paths")?
There was a problem hiding this comment.
Do we want to crash the daemon on this? That seems a little drastic
There was a problem hiding this comment.
I think so. These paths are coming from filewatching, right? If that system isn't giving us what we expect, it's a programming error on our part and lots of things will be broken. I think in that scenario, crashing is the right thing to do.
| }) | ||
| .filter(|p| { | ||
| // If in .gitignore or in .git, filter out | ||
| !(ancestors_is_ignored(&root_gitignore, p) || is_in_git_folder(p)) |
There was a problem hiding this comment.
I suspect that at some point this'll be an issue w/ inputs.
There was a problem hiding this comment.
yeah there's a subsequent PR that hooks up file hashing and removes this check
NicholasLYang
force-pushed
the
nicholasyang/package-change-watcher-perf
branch
from
d510c4c
to
c19392a
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @NicholasLYang and the rest of your teammates on |
…cel#8064) ### Description Like vercel#8036, we need to split out the event processing loop from the actual logic to avoid lagging channels. In this case we process the changed files every 100ms. Uses a `Trie` to store the paths since they'll likely have a lot of overlap. ### Testing Instructions <!-- Give a quick description of steps to test your changes. --> Closes TURBO-2921
Description
Like #8036, we need to split out the event processing loop from the actual logic to avoid lagging channels. In this case we process the changed files every 100ms.
Uses a
Trieto store the paths since they'll likely have a lot of overlap.Testing Instructions
Closes TURBO-2921