Skip to content

Conversation

@eddyb
Copy link
Member

@eddyb eddyb commented Dec 18, 2024

We've never added any automation for this so it keeps drifting.

The core issue here looks like this (after running all these commands at least once):

$ cargo build -p compiletests --release --no-default-features --features use-installed-tools
   Compiling rustc_codegen_spirv v0.9.0 (/home/eddy/Projects/rust-gpu/crates/rustc_codegen_spirv)
   Compiling compiletests v0.0.0 (/home/eddy/Projects/rust-gpu/tests)
    Finished `release` profile [optimized] target(s) in 2.92s
$ cargo build -p example-runner-wgpu --release --no-default-features --features use-installed-tools
   Compiling rustc_codegen_spirv v0.9.0 (/home/eddy/Projects/rust-gpu/crates/rustc_codegen_spirv)
   Compiling spirv-builder v0.9.0 (/home/eddy/Projects/rust-gpu/crates/spirv-builder)
   Compiling example-runner-wgpu v0.0.0 (/home/eddy/Projects/rust-gpu/examples/runners/wgpu)
    Finished `release` profile [optimized] target(s) in 3.58s
$ cargo build -p compiletests --release --no-default-features --features use-installed-tools
   Compiling rustc_codegen_spirv v0.9.0 (/home/eddy/Projects/rust-gpu/crates/rustc_codegen_spirv)
   Compiling compiletests v0.0.0 (/home/eddy/Projects/rust-gpu/tests)
    Finished `release` profile [optimized] target(s) in 2.64s
$ cargo build -p example-runner-wgpu --release --no-default-features --features use-installed-tools
   Compiling rustc_codegen_spirv v0.9.0 (/home/eddy/Projects/rust-gpu/crates/rustc_codegen_spirv)
   Compiling spirv-builder v0.9.0 (/home/eddy/Projects/rust-gpu/crates/spirv-builder)
   Compiling example-runner-wgpu v0.0.0 (/home/eddy/Projects/rust-gpu/examples/runners/wgpu)
    Finished `release` profile [optimized] target(s) in 3.44s

That is, rustc_codegen_spirv gets rebuilt for different downstream uses of it.

While duplicating the initial build work would be unfortunate on its own, it's much worse to also have rebuild friction just from going back & forth between examples/tools.
(I believe this is worse than for the average crate, because Cargo can't allow N simultaneous copies of librustc_codegen_spirv-*.so - the usual hash suffix would interfere with dylib usage, and this also applies to almost everything other than rlibs, e.g. executables are also built without a hash suffix etc.)


After this PR (after running all these commands at least once):

$ cargo build -p compiletests --release --no-default-features --features use-installed-tools
    Finished `release` profile [optimized] target(s) in 0.11s
$ cargo build -p example-runner-ash --release --no-default-features --features use-installed-tools
    Finished `release` profile [optimized] target(s) in 0.13s
$ cargo build -p example-runner-wgpu --release --no-default-features --features use-installed-tools
    Finished `release` profile [optimized] target(s) in 0.16s
$ cargo build -p compiletests --release --no-default-features --features use-installed-tools
    Finished `release` profile [optimized] target(s) in 0.11s
$ cargo build -p example-runner-ash --release --no-default-features --features use-installed-tools
    Finished `release` profile [optimized] target(s) in 0.13s
$ cargo build -p example-runner-wgpu --release --no-default-features --features use-installed-tools
    Finished `release` profile [optimized] target(s) in 0.15s

This is accomplished in two major ways:

  • reducing version duplication (via cargo deny enforcement)
    • this required most of the code changes, because example-runner-ash was on the previous version of ash, and the upgrade for breaking changes was pretty involved
      (including fixing previously-UB usage of Box<T> being moved while also passing around &T borrows hidden via *const T in Vulkan structs)
    • sadly some exceptions remain, due to the ecosystem itself not having updated various common dependencies
    • one big source of exceptions is from outdated winit (will open as a separate PR)
  • unifying feature sets via crates/rustc_codegen_spirv/Cargo.toml "phony" deps
    • originally started in Avoid ever building rustc_codegen_spirv more than once in release mode. EmbarkStudios/rust-gpu#546
    • hard to automate (consider looking into cargo-hikari etc.?)
    • ideally only (transitive) dependencies of rustc_codegen_spirv would be compared
    • I did come up with one technique to start tracking this down:
      (click to expand ridiculous hack)
      echo compiletests example-runner-{ash,wgpu} | xargs -n1 bash -c 'cargo build -p "$0" --release --no-default-features --features use-installed-tools -Z unstable-options --unit-graph' | jq -s 'map(.units | map(del(.dependencies)) | group_by(.pkg_id) | map({key:.[0].pkg_id, value:.}) | from_entries) as $P | ([$P[] | keys[]] | unique | map(select(. as $k | $P | map(select(has($k))) | length > 1))) as $shared | ($shared | map({key:.,value:[$P[][.]]}) | from_entries) | map_values(transpose | map(select(map(objects) | unique | length > 1)) | map(if (map(objects | del(.features)) | unique | length) == 1 then map(objects | .features | "features: \(join("+"))") else . end) | unique | select(length > 0))'

      that "one-liner" would output JSON like this (at least for compiletests vs example-runner-wgpu, before starting to make feature-unifying changes):
      {
        "ahash 0.8.11 (registry+https://github.com/rust-lang/crates.io-index)": [
          [
            "features: ",
            "features: default+getrandom+no-rng+runtime-rng+std"
          ]
        ],
        "bytemuck 1.20.0 (registry+https://github.com/rust-lang/crates.io-index)": [
          [
            "features: ",
            "features: aarch64_simd+bytemuck_derive+derive"
          ]
        ],
        "linux-raw-sys 0.4.14 (registry+https://github.com/rust-lang/crates.io-index)": [
          [
            "features: elf+errno+general+ioctl+no_std",
            "features: elf+errno+general+if_ether+ioctl+net+netlink+no_std+prctl+system+xdp"
          ]
        ],
        "log 0.4.22 (registry+https://github.com/rust-lang/crates.io-index)": [
          [
            "features: ",
            "features: std"
          ]
        ],
        "rustix 0.38.42 (registry+https://github.com/rust-lang/crates.io-index)": [
          [
            "features: alloc+default+fs+libc-extra-traits+std+use-libc-auxv",
            "features: alloc+default+event+fs+libc-extra-traits+net+pipe+process+shm+std+system+thread+time+use-libc-auxv"
          ]
        ],
        "smallvec 1.13.2 (registry+https://github.com/rust-lang/crates.io-index)": [
          [
            "features: serde+union",
            "features: const_generics+const_new+serde+union"
          ]
        ]
      }

Copy link
Collaborator

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

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

Lgtm

@LegNeato LegNeato added this pull request to the merge queue Dec 18, 2024
Merged via the queue into Rust-GPU:main with commit 211b7fc Dec 18, 2024
7 checks passed
@eddyb eddyb deleted the cg-spv-build-dedup branch December 18, 2024 16:05
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