feat(core): use rust utilities for caching operations #17638
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
I added walkdir and alphabetized the list
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 6e5787e. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
Cammisuli
force-pushed
the
rust_cache_support
branch
from
38de55e
to
8790b71
Compare
Cammisuli
force-pushed
the
rust_cache_support
branch
from
ec18ff4
to
bdd0622
Compare
| @@ -83,7 +71,7 @@ export class Cache { | |||
| const tdCommit = join(this.cachePath, `${task.hash}.commit`); | |||
|
|
|||
| // might be left overs from partially-completed cache invocations | |||
| await remove(tdCommit); | |||
| await this.remove(tdCommit); | |||
There was a problem hiding this comment.
Does this yield performance benefits? 👀
There was a problem hiding this comment.
It doesnt use the node remove anymore, and the rust one appears to be faster.
| pub fn expand_outputs(directory: String, entries: Vec<String>) -> anyhow::Result<Vec<String>> { | ||
| let directory: PathBuf = directory.into(); | ||
|
|
||
| let (existing_paths, not_found): (Vec<_>, Vec<_>) = entries.into_iter().partition(|entry| { |
There was a problem hiding this comment.
Can we rename not_found to globs?
| res(); | ||
| } catch (e) { | ||
| rej(e); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| private async remove(path: string): Promise<void> { |
There was a problem hiding this comment.
followup: Can we make all these methods sync?
| crossbeam-channel = '0.5' | ||
|
|
||
| fs_extra = "1.3.0" |
There was a problem hiding this comment.
This is new too right?
| pub fn remove(src: String) -> anyhow::Result<()> { | ||
| fs_extra::remove_items(&[src]).map_err(anyhow::Error::from) | ||
| } |
There was a problem hiding this comment.
I don't see a reason why we wouldn't expose the ability to remove multiple. In theory, it makes no difference but it's a little cleaner I guess.
Is there a way to get the js signature to be ...srcs: string[]?
At the very least, we can make
Cache.remove(...srcs: string[]) {
require('../native').remove(srcs);
}
| pub fn remove(src: String) -> anyhow::Result<()> { | |
| fs_extra::remove_items(&[src]).map_err(anyhow::Error::from) | |
| } | |
| pub fn remove(srcs: Vec<String>) -> anyhow::Result<()> { | |
| fs_extra::remove_items(&[src]).map_err(anyhow::Error::from) | |
| } |
packages/nx/Cargo.toml
Outdated
| crossbeam-channel = '0.5' | ||
|
|
||
| fs_extra = "1.3.0" | ||
| futures = "0.3.28" |
Cammisuli
force-pushed
the
rust_cache_support
branch
from
26dbc25
to
c6fccb8
Compare
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
Nx cache uses fast-glob, fs, and execSync to manage cache. (Finding what's needed to cache, copy and remove)
Expected Behavior
Nx cache uses rust utilities to manage these operations
Related Issue(s)
Fixes #