Skip to content

perf(spirv-builder): resolve the target dir once per SpirvWatcher - #695

Closed
sorinirimies wants to merge 1 commit into
Rust-GPU:mainfrom
sorinirimies:perf/spirv-watcher-resolve-target-dir-once
Closed

sorinirimies wants to merge 1 commit into
Rust-GPU:mainfrom
sorinirimies:perf/spirv-watcher-resolve-target-dir-once

Conversation

@sorinirimies

Copy link
Copy Markdown

Summary

With the default relative target_dir_path ("spirv-builder"), invoke_rustc runs cargo metadata to find the target dir on every call — so every rebuild of a SpirvWatcher.

 SpirvWatcher::recv_inner
+  pin_target_dir()                 // once: relative -> absolute via cargo metadata;
+                                   // on failure: debug log, retried next build
   builder.invoke_rustc()
-    cargo metadata                 // every rebuild
+    resolve_target_dir()           // absolute now -> no subprocess

SpirvBuilder::resolve_target_dir is extracted from invoke_rustc unchanged.

Evidence

cargo metadata cost saved per rebuild (sky-shader, warm):

hyperfine -N 'cargo metadata --format-version 1'
  Time (mean ± σ):  228.4 ms ± 5.4 ms   [15 runs]

New tests::target_dir:

absolute_target_dir_is_kept                       ok   // no cargo metadata needed
relative_target_dir_is_inside_cargo_target_dir    ok

cargo test -p spirv-builder --features watch,clap: all pass. Clippy -D warnings clean.

Merge Danger

Door: two-way

Blast Radius: small — watcher only. If a user changes the cargo target dir (e.g. .cargo/config.toml build.target-dir) while watching, the watcher keeps the old one until restarted.

A relative `target_dir_path` (the default) is resolved with
`cargo metadata` on every `invoke_rustc`, i.e. on every rebuild of a
watched crate (~230ms for sky-shader). `SpirvWatcher` now pins it to the
absolute path before the first build; failures are retried on the next
build and reported by `invoke_rustc` as before.

Extracts `SpirvBuilder::resolve_target_dir` and tests it.
@Firestar99 Firestar99 closed this Nov 7, 2026
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