ash icon indicating copy to clipboard operation
ash copied to clipboard

ash: Add `const STRUCTURE_TYPE` to all Vulkan structures for matching with `match_struct!` macro

Open MarijnS95 opened this issue 3 years ago • 5 comments

Depends on #613

In Vulkan layers extracing a structure based on its s_type is a common operation, but comparing against an enum value and subsequently casting to the right type is verbose and error-prone.

By generating a const STRUCTURE_TYPE with the given value for every Vulkan structure it becomes possible to implement a macro that abstracts this logic away in a safer way.

MarijnS95 avatar Apr 15 '22 14:04 MarijnS95

I'll test and post some build-timings later, see if +2300 trivial trait implementation lines have any effect. As I needed this while writing some Vulkan layers in Rust, the goal is to also submit a minimal layer crate for ash at some point, initially providing just some types before adding better utilities. We'll need to publicize ptr_chain_iter in const and mut variants too.

MarijnS95 avatar Apr 15 '22 14:04 MarijnS95

I'll test and post some build-timings later, see if +2300 trivial trait implementation lines have any effect.

Easy enough to cfg-gate if needed, since it's a new trait.

Ralith avatar Apr 15 '22 18:04 Ralith

master @ 61ab543d:

$ hyperfine -p 'cargo clean -p ash' 'cargo b -p ash'
Benchmark 1: cargo b -p ash
  Time (mean ± σ):      7.366 s ±  0.101 s    [User: 8.313 s, System: 0.670 s]
  Range (min … max):    7.248 s …  7.586 s    10 runs

match-structure-type @ eb95efb:

$ hyperfine -p 'cargo clean -p ash' 'cargo b -p ash'
Benchmark 1: cargo b -p ash
  Time (mean ± σ):      7.411 s ±  0.065 s    [User: 8.358 s, System: 0.638 s]
  Range (min … max):    7.305 s …  7.505 s    10 runs

We're talking approx 50ms here, nothing worth cfg()'ing nor worrying for.

MarijnS95 avatar Apr 15 '22 23:04 MarijnS95

@Ralith How do you feel about a hyperfine step for the core crate in CI?

MarijnS95 avatar Apr 15 '22 23:04 MarijnS95

I've heard CI tends to give erratic timings for CPU-bound stuff due to contention, but it might be interesting to try all the same.

Ralith avatar Apr 15 '22 23:04 Ralith

@jdtatz only noticing https://github.com/jdtatz/ash/commits/match-structure-type-more-generic just now; any reason this wasn't suggested as a change to cut down on codegen for push_next()? What's the (hopefully positive) compile time improvement?

Or is this all blocking on the recently submitted type support in vk-parse so that we can subsequently drop vkxml use in ash's generator and finally use the validstructs property, to keep push_next() for pNext versus validstructs fields apart so that we can - in the end - get https://github.com/ash-rs/ash/pull/622 in with a proper binding?

MarijnS95 avatar Sep 20 '22 10:09 MarijnS95