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

[naga] Compaction should not require cloning the entire type arena #6789

Closed
jimblandy opened this issue Dec 19, 2024 · 3 comments · Fixed by #6790
Closed

[naga] Compaction should not require cloning the entire type arena #6789

jimblandy opened this issue Dec 19, 2024 · 3 comments · Fixed by #6790
Labels
area: naga middle-end Intermediate representation area: performance How fast things go kind: refactor Making existing function faster or nicer naga Shader Translator

Comments

@jimblandy
Copy link
Member

The Module compaction code in compact::compact clones the module's entire type arena just to fix up the Expression handles of arrays whose sizes are override expressions. This clone really shouldn't be necessary. It should be possible to handle this when we drop unused types from the type arena, earlier in the function.
@jimblandy jimblandy added area: performance How fast things go naga Shader Translator kind: refactor Making existing function faster or nicer area: naga middle-end Intermediate representation labels Dec 19, 2024
@jimblandy jimblandy changed the title [naga] Compaction should not require cloning the entire type arena. [naga] Compaction should not require cloning the entire type arena Dec 19, 2024
@jimblandy
Copy link
Member Author

cc @kentslaney

@jimblandy
Copy link
Member Author

@kentslaney Since I'm complaining so much here and in #6788, I should also say: thank you very much for #6635, #6654, and #6746! These are important features. Because they are so thorny, we should have tackled them sooner. I'm not really surprised there have been wrinkles.

@jimblandy
Copy link
Member Author

I may have a fix for this...

jimblandy added a commit to jimblandy/wgpu that referenced this issue Dec 19, 2024
@jimblandy
[naga] In compaction, fix array lengths as part of type adjustment.
c43e852 
Adjust the `Handle<Expression>` values that appear in
`TypeInner::Array` via `PendingArraySize::Expression` as part of the
normal type adjustment process in `ModuleMap::adjust_type`, rather
than cloning the type arena so we can iterate over it and call
`UniqueArena::replace`.

Fixes gfx-rs#6789.
@jimblandy jimblandy mentioned this issue Dec 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: naga middle-end Intermediate representation area: performance How fast things go kind: refactor Making existing function faster or nicer naga Shader Translator
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[naga] In compaction, fix array lengths as part of type adjustment. jimblandy/wgpu
1 participant
@jimblandy