Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Shader compile times #3389

Open
kvark opened this issue Sep 28, 2020 · 2 comments
Open

Shader compile times #3389

kvark opened this issue Sep 28, 2020 · 2 comments
Labels
module: infrastructure status: ready for work type: optimization value: medium

Comments

@kvark
Copy link
Member

kvark commented Sep 28, 2020

Using spirv-cross appears to be showing up in profiles. We are doing a bit of heap allocation and copying on the way to it and from it today, but also the logic itself may be slow. Hopefully, we can address it by moving to https://github.com/gfx-rs/naga

Here is some input I gathered from a Dota2 run:
dota-metal-shader-times.txt

All times are in micro-seconds. Here are some observations:

  • spirv-cross time just dominates all the other metrics. It's really a big deal to replace/optimize
  • pipeline creation is comparable to MTLLibrary creation of a single stage. It's not much at all in the total picture.
@kvark kvark mentioned this issue Sep 28, 2020
Closed
@mkeeter
Copy link

mkeeter commented Jan 20, 2021

It turns out there was accidentally exponential behavior in SPIRV-Cross – not sure if this was being hit, but it's a possibility!

The spirv_cross crate has updated to the latest upstream in 0.23.0. Depending on when naga will be ready, maybe it's worth bumping the spirv_cross dependency for a point release? I'd be happy to make a PR and see if anything breaks, though I don't have a full set of cross-platform hardware for testing.

(I'm biased, because I'm running into this behavior in an app using wgpu-native, so I'm trying to push the fix all the way downstream 😄)

@grovesNL grovesNL mentioned this issue Jan 21, 2021
Merged
@bors bors bot closed this as completed in 2d454c7 Jan 21, 2021
@grovesNL
Copy link
Contributor

Accidentally closed

@grovesNL grovesNL reopened this Jan 21, 2021
@kvark kvark mentioned this issue Feb 15, 2021
Merged
bors bot added a commit that referenced this issue Feb 16, 2021
@bors @kvark
Merge #3641
cef0aab 
3641: Make spirv-cross optional r=grovesNL a=kvark

SPIRV-Cross is a big foreign dependency, which often cause issues, such as denoland/rusty_v8#465 and #3389, #3117, #2520, and many others.
Previously in #3445 and #3520 we implemented Naga code path as an alternative, but hidden behind an optional feature. Now it's time to flip the table and say that Naga is permanent, but SPIRV-Cross is optional until completely phased out.

Most of the changes here are from re-routing the main shader creation logic from using SPIRV-Cross types to using Naga types. The "cross" stuff is just getting derived from Naga types if necessary.

The change only applies to Metal and OpenGL backends so far. D3D will come on board once gfx-rs/naga#408 is operational.

Note: it's going to be a long while for Naga to mature, most users need to keep the "cross" dependency enabled for now! Even the included examples don't run on Naga yet, since the SPIR-V generated by glslang is quite specific, and Naga's SPV frontend doesn't like it yet.

Co-authored-by: Dzmitry Malyshau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
module: infrastructure status: ready for work type: optimization value: medium
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kvark @mkeeter @grovesNL