Skip to content

Conversation

@kotauskas
Copy link

Inlining the destructor of ErrorData (which currently gets inlined through the destructor of the packed Repr) is in no way helpful in real programs, as the source of the error will not be inlined, so there will not be any match assumptions to gain. The cost, meanwhile, is a code size increase by a factor of up to 5.4 in the case of dropping multiple io::Results in the same function. Accordingly, this disables the inlining to avoid unhelpful code bloat in opt-level = 3 programs.

The destructor of ErrorData on 32-bit platforms might be suffering from the same problem, but fixing that would require some sort of annotation that puts #[inline(never)] on the compiler-generated part of the destructor.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 20, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@joboet
Copy link
Member

joboet commented Nov 20, 2025

I don't think preventing inlining of the whole function is a good idea. For cases where the compiler knows the error variant, inlining allows removing the entire destructor. I think it'd be better to prevent inlining the destructor of the custom variant, which is the only one that actually needs destruction.

@kotauskas
Copy link
Author

For cases where the compiler knows the error variant, inlining allows removing the entire destructor.

Those are highly unlikely to occur on hot paths in real programs: the construction point of the error would have to get inlined into the destruction point. It's most certainly not worth inlining the branches (which still worsen the code size, albeit not as much as inlining the destructor) into every io::Error destruction point in hopes that a handful of them will be slightly faster in the particular case of someone having stubbed out a bunch of functions that return io::Result with ones that always return Err(io::Error::from(io::ErrorKind::Unsupported)). Especially the fact that this only happens when stubbing on #[cfg] makes it very unlikely for the destructor elision made possible by inlining the branches to yield meaningful improvements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants