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.

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

GLES: Cache and reuse programs between similar pipelines #3380

Merged
merged 5 commits into from
Jan 20, 2023

Conversation

Dinnerbone
Copy link
Contributor

@Dinnerbone Dinnerbone commented Jan 14, 2023

Checklist

  • Run cargo clippy.
  • Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections
None

Description
It's a common practice to create many, many pipelines due to a lot of state being "per pipeline" and not adjustable on the fly, such as blend modes. This turns them into an often giant explosion of permutations for shaders vs blend vs depth operations (and other things). As the GL pipeline unfortunately needs to block on the creation of each program, and we create one program per pipeline, this can lead to stalls over 10 seconds long as we create the pipelines needed for our application.

This change decouples a Pipeline from a GL program, and allows sharing of the same program across many pipelines. As long as the inputs are the same, the program created will always be the same and thus can be shared safely.

For ruffle with webgl, this reduces a startup time from about 10 seconds to almost instantaneous due to the amount of programs we create, for a very few combinations of shaders.

Testing
All the existing tests pass and I've monitored that programs do indeed get destroyed when every pipeline that uses them is deleted.

I've manually tested Ruffle with this change to confirm the speedup, and it's fast and stable.

I haven't created any new tests, I'm not sure how to test this automatically.

@codecov-commenter
Copy link

codecov-commenter commented Jan 14, 2023

Codecov Report

Merging #3380 (6c452f3) into master (4400ff8) will increase coverage by 0.04%.
The diff coverage is 96.29%.

@@            Coverage Diff             @@
##           master    #3380      +/-   ##
==========================================
+ Coverage   64.64%   64.68%   +0.04%     
==========================================
  Files          86       86              
  Lines       42722    42780      +58     
==========================================
+ Hits        27616    27673      +57     
- Misses      15106    15107       +1     
Impacted Files Coverage Δ
wgpu-hal/src/gles/device.rs 82.19% <96.15%> (+0.78%) ⬆️
wgpu-hal/src/gles/adapter.rs 83.77% <100.00%> (+0.04%) ⬆️
wgpu-hal/src/gles/mod.rs 61.83% <100.00%> (+0.29%) ⬆️
wgpu-core/src/hub.rs 61.29% <0.00%> (+0.15%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Great idea, good execution, have some questions/comments but looking forward to getting this in.

wgpu-hal/src/gles/device.rs Outdated Show resolved Hide resolved
wgpu-hal/src/gles/device.rs Show resolved Hide resolved
wgpu-hal/src/gles/device.rs Outdated Show resolved Hide resolved
wgpu-hal/src/gles/mod.rs Outdated Show resolved Hide resolved
wgpu-hal/src/gles/device.rs Show resolved Hide resolved
@cwfitzgerald
Copy link
Member

You re-requested review but didn't push anything 😄

@Dinnerbone
Copy link
Contributor Author

That's... huh. Where the hell am I pushing things?!
Sorry about that, one moment!

@Dinnerbone
Copy link
Contributor Author

I had a bunch of commits since I made the PR and apparently I've been pushing them to the wrong place. I'm juggling too many things 😅

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

G2G after nit

wgpu-hal/src/gles/device.rs Outdated Show resolved Hide resolved
@cwfitzgerald
Copy link
Member

Thanks for all your help on the GL backend!

@yudshj
Copy link

yudshj commented May 28, 2023

Hello, may I ask if it’s possible to cache and reuse similar pipelines based on a similar approach, rather than just caching the program? This would help save time in assembling WebGL pipelines.

@Wumpf
Copy link
Member

Wumpf commented May 29, 2023

It sure could be done, but the idea here is that only the program creation is the actual slow part. Everything else is mostly just ephemeral state that needs to be set during rendering, so I don't think there is any speedup to be gained.

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.

6 participants