Reduce bloat from wgsl::Error::as_parse_error by bolshoytoster · Pull Request #9944 · gfx-rs/wgpu

bolshoytoster · 2026-07-25T20:57:54Z

Connections
#9941

Description
wgsl::Error::as_parse_error causes a lot of bloat in binary size and LLVM IR. This PR makes some changes to reduce the amount of bloat.

More info in #9941

Testing
I've checked that the LLVM IR and binary outputs are lower with llvm-lines and cargo-bloat
TODO: run all the tests

Squash or Rebase?

Squash (this is a draft)

Checklist

I self-reviewed and fully understand this PR.
WebGPU implementations built with wgpu may be affected behaviorally.
Validation and feature gates are in place to confine behavioral changes.
Tests demonstrate the validation and altered logic works.
CHANGELOG.md entries for the user-facing effects of this change are present.
The PR is minimal, and doesn't make sense to land as multiple PRs.
Commits are logically scoped and individually reviewable.
The PR description has enough context to understand the motivation and solution implemented.

bolshoytoster · 2026-07-25T21:08:46Z

The current output size is 4302 lines of IR, 18.3KiB of output. Note that this is larger that what I claimed in the linked issue, since I was testing with 29.0.3 there, and this is aimed at main.

In addition to the changes discussed in the issue, this also reorders some of the fields when constructing ParserErrors. This results in less IR, due to the fact that the compiler was inserting Cow::drops even when it knew they were Cow::Borrowed, e.g.:

ParseError {
	// This creates a Cow::Borrowed
	message: "unexpected components".into(),
	// vec! can panic if allocating fails, so it needs to generate code to drop everything in scope.
	// It generates Cow::drop for message, even though it's not needed
	labels: vec![(bad_span, "unexpected components".into())],
	notes: vec![],
}

A better version:

ParseError {
	// This can still panic, but now it doesn't have to drop message
	labels: vec![(bad_span, "unexpected components".into())],
	message: "unexpected components".into(),
	notes: vec![],
}

bolshoytoster · 2026-07-25T21:40:29Z

I have been able to get it down to 3485 lines and 16.3KiB locally by using my own implementation of SmallVec (requires unsafe).

The reason I did this was to find out the maximum size that ould be saved by SmallVec. Unfortunately, using the actual SmallVec increases the output size, seemingly because the smallvec! macro is inefficient (for the inline case, it's implemented by creating an empty one and repeatedly pushing to it).

I'm going to test to see if it's better with smallvec 2.0.0-alpha.12, and if not, I'll look into contributing there to make the macro more efficient. (I don't think naga allows unsafe code, so I can't just use my minimal implementation here).

Edit: I've got it to 3389 and 16.0KiB by using SmallVec::from_buf, which can only be used like this in 2.0.0. So it can't really be used unless wgpu decides to use an 'alpha' version of smallvec, or if smallvec finally releases v2 (unlikely to be soon, since the v2 branch hasn't been touched for 5 months)

kpreid · 2026-07-26T00:54:21Z

                        Token::Attribute => "@".to_string(),
                        Token::Number(_) => "number".to_string(),
                        Token::Word(s) => s.to_string(),


Does making expected_str a Cow so it doesn’t have many separate to_string() calls help?

In general, when trying to reduce binary size, I’ve found that it’s useful to push as many things as possible into being literals instead of repeated function calls, so I think expressing these as Cow::Borrowed("@") is worth trying (or even const { Cow::Borrowed("@") }, if necessary).

For some reason when I tried that here it actually made the output bigger. I'll try using const {}.

Using const {} helped, but the output is still bigger than without Cow :(

bolshoytoster · 2026-07-26T20:54:33Z

I think I've gotten it as low as I can (3775, 17.4KiB). If I'm allowed to update smallvec to 2.0.0-alpha.12 (probably not), then it could be 3206 and 14.2KiB. Unfortunately, using the current version of smallvec increases the size to 3960 21.1KiB.

The latest commit separates part of the function out and merges some of the format!s and .to_string()s. It also avoids dereferencing &self in as_parse_error, because that helps for some reason.

It also merges some duplicate &source[span]s, since the compiler wasn't merging them automatically.

Notes in case anyone wants to try to squeeze it further:
Most of the generated code is drop glue inserted due to functions that can panic:

71 from format!, since it allocates a String, and allocation can fail (maybe you could replace them with write!ing into a preallocated (probably on the stack, and with enough capacity to handle pretty much any string) buffer, and truncates/aborts if it overruns)
12 from &source[span]

bolshoytoster · 2026-07-27T08:16:07Z

Possibly related to #9052 and #9075: since you sometimes can't precompile shaders (if you don't know what API you might be using at compile time) would it be worth adding a feature to strip down things like this (i.e. in this case, the feature could cause as_parse_error to just return a generic error, telling you that you need to enable the feature to see errors).

kpreid

Overall feedback: this needs more comments. Any time one writes code weirdly for the sake of a performance characteristic, one should document the reason for the weirdness and how the code should be maintained under this constraint.

kpreid · 2026-07-30T16:14:20Z

 }

+impl ExpectedToken<'_> {
+    fn as_string(&self) -> String {


Since this constructs a string, it should be named to_string(). But relatedly, have you tried implementing Display instead? I wouldn’t be surprised if that’s worse, but it’s worth checking (and commenting).

I originally considered implementing Display, but opted not to. I named it as_string to avoid the clippy lint telling me to implement Display.

I've implemented Display now, and the output is smaller now that I can avoid calling to_string.

kpreid · 2026-07-30T16:15:45Z

+            FormatChar(&'static str, char, &'static str),
+            FormatStr([&'a str; 3]),
+        }
+        match match self {


Does this have to be a match match? I would expect that adding a let doesn’t significantly affect the output, and will be clearer.

I've moved out one of the matched into a variable.

kpreid · 2026-07-30T16:20:24Z

-                message: format!("invalid field accessor `{}`", &source[accessor_span],),
-                labels: vec![(accessor_span, "invalid accessor".into())],
-                notes: vec![],
+            // Other variants with a static message, single label and no notes


This comment is inaccurate — not all of the messages are static.

Whoops, I think I meant 'Other variants with a single static label and no notes'

kpreid · 2026-07-30T16:20:59Z

 }

+impl ExpectedToken<'_> {
+    fn as_string(&self) -> String {


This function needs a comment (not a doc-comment but an // comment) explaining why it is written so non-straightforwardly, and perhaps how to validate future changes to it.

I've added comments here and to as_parse_error, but I'm not sure how well they explain things.

kpreid · 2026-07-30T16:24:19Z

-                message: format!("no definition in scope for identifier: `{ident}`"),
-                labels: vec![(ident_span, "unknown identifier".into())],
+            Error::BadNumber(bad_span, ref err) => ParseError {
+                message: format!("{}: `{}`", err, &source[*bad_span]).into(),


Personally, I would write Cow::Borrowed (or Borrowed with use Cow::Borrowed;) instead of using .into() for these conversions — it’s more explicit (doesn’t leave the reader wondering “why are we making a string and immediately converting it?") and might be faster to compile (I'm just guessing, but it doesn’t require a trait implementation lookup).

But this is likely not a big deal either way.

I used .into() because the original code was using it, I think having Cow::Borrowed might be a bit verbose.

Lower bloat from wgsl::Error::as_parse_error

8d9d7ee

kpreid reviewed Jul 26, 2026

View reviewed changes

More squeezing

767943b

bolshoytoster marked this pull request as ready for review July 28, 2026 08:55

Wumpf assigned kpreid Jul 29, 2026

kpreid requested changes Jul 30, 2026

View reviewed changes

bolshoytoster added 2 commits July 30, 2026 20:01

Improve comments and implement Display for ExpectedToken

a5d190d

Fix typo

6a000e6

Uh oh!

Conversation

bolshoytoster commented Jul 25, 2026

Uh oh!

bolshoytoster commented Jul 25, 2026

Uh oh!

bolshoytoster commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bolshoytoster commented Jul 26, 2026

Uh oh!

bolshoytoster commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kpreid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bolshoytoster Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bolshoytoster commented Jul 25, 2026 •

edited

Loading

bolshoytoster commented Jul 27, 2026 •

edited

Loading

bolshoytoster Jul 30, 2026 •

edited

Loading