Reduce bloat from `naga::Error::as_parse_error`

[bolshoytoster]

I currently measure naga::Error::as_parse_error to emit 7818 lines of LLVM IR, taking up 27.9KiB in the output binary (using llvm-lines and cargo-bloat). I have managed to get it down to ~5119 lines of LLVM IR and ~22.2KiB of output by merging a lot of the match arms that are similar, i.e. changing

Error::UnexpectedComponents(bad_span) => ParseError {
	message: "unexpected components".to_string(),
	labels: vec![(bad_span, "unexpected components".into())],
	notes: vec![],
},
Error::UnexpectedOperationInConstContext(span) => ParseError {
	message: "this operation is not supported in a const context".to_string(),
	labels: vec![(span, "operation not supported here".into())],
	notes: vec![],
},
Error::NotStorageTexture(bad_span) => ParseError {
	message: "textureStore can only be applied to storage textures".to_string(),
	labels: vec![(bad_span, "not a storage texture".into())],
	notes: vec![],
},
// ..

to

Error::UnexpectedComponents(span)
	| Error::UnexpectedOperationInConstContext(span)
	| Error::NotStorageTexture(span)
	// ..
	=> {
	let (message, label) = match self {
		Error::UnexpectedComponents(_) => (
			"unexpected components",
			"unexpected components"
		),
		Error::UnexpectedOperationInConstContext(_) => (
			"this operation is not supported in a const context",
			"operation not supported here"
		),
		Error::NotStorageTexture(_) => (
			"textureStore can only be applied to storage textures",
			"not a storage texture"
		),
		// ..
		_ => unreachable!()
	};

	ParseError {
		message: message.to_string(),
		labels: vec![(span, label.into())],
		notes: vec![],
	}
}

And avoiding calling .to_string() in the Error::Unexpected arm:

Token::Separator(c) => format!("`{c}`"),
Token::Paren(c) => format!("`{c}`"),
Token::Attribute => "@".to_string(),
Token::Number(_) => "number".to_string(),
Token::Word(s) => s.to_string(),

->

Token::Separator(c) | Token::Paren(c) => Cow::Owned(format!("`{c}`")),
Token::Attribute => Cow::Borrowed("@"),
Token::Number(_) => Cow::Borrowed("number"),
Token::Word(s) => Cow::Borrowed(s),

There are probably more opportunities to reduce the function's size, but a 35% reduction in IR isn't nothing.

Would this be accepted as a PR?

Edit: I've now gotten it down to 4880 lines IR, 21.4KiB output 4501 lines IR, 18.5KiB output

It looks like most of the generated IR is drop glue (since the function uses a lot of functions that allocate, which can panic). It seems to generate ~154 separate almost-identical unwind blocks. I'm not sure how to convince the compiler to merge these.

If anyone knows more about this, I'd like some help to see if the function can be made even smaller.

[kpreid]

Would this be accepted as a PR?

Yes, please! We like it when naga is smaller.

The one concern I have is costs to maintainability via your proposed use of requiring two levels of match to fit each other via unreachable!(), but with appropriate comments, within a single function, that shouldn’t be a significant problem.

It looks like most of the generated IR is drop glue (since the function uses a lot of functions that allocate, which can panic). It seems to generate ~154 separate almost-identical unwind blocks. I'm not sure how to convince the compiler to merge these.

Looking at the code, one idea that comes to mind immediately is to try expressing the labels using an ArrayVec or SmallVec so that most constructions of it don’t allocate, at least until a final conversion into the ParseError type. That still leaves miscellaneous String allocations, but we could see if temporarily keeping them as Cow<str> helps. I’d be interested in poking at this myself — after you’ve shared your work.

[bolshoytoster]

but we could see if temporarily keeping them as Cow<str> helps.

I've seen a ~20% additional improvement by just changing the Strings in ParseError to Cows (down to 4063 IR, 16.5KiB). This shouldn't be a breaking change since the fields are private, although I think it might make the struct slightly bigger. This will also avoid calling .to_string() on a few string literals, although performance doesn't really matter here.

try expressing the labels using an ArrayVec or SmallVec so that most constructions of it don’t allocate

I tried using ArrayVec earlier, but it actually made the output bigger, although it may be different now that I've reduced allocations from the Strings. The problem is that using ArrayVecs temporarily here wouldn't help reduce allocations, since they need to be converted to a Vec anyway, i.e. it would just be:

Error::UnexpectedComponents(bad_span) => ParseError {
	message: "unexpected components".into(),
	labels: ArrayVec::from([(bad_span, "unexpected components".into())]).to_vec(),
	notes: vec![],
},

I also tried to construct the ParseError outside the match, i.e.

let (message, labels, notes) = match *self {
	Error::UnexpectedComponents(bad_span) => (
		"unexpected components".into(),
		ArrayVec::from([(bad_span, "unexpected components".into())]),
		ArrayVec::new(),
	)
};

ParseError {
	message,
	labels: labels.to_vec(),
	notes: notes.to_vec()
}

But the compiler seemed to compile that to the same thing as previous, even when I tried putting black_boxes everywhere,

[kpreid]

Well, I don’t have any further ideas that aren’t direct experimentation. I’d like to see your PR, and then I can think about how to improve on that.

[bolshoytoster]

I've opened a draft PR with my current progress on this.