Skip to content

Each usage of {:?} on usize appears to generate a separate adapater to core::fmt::Display #47785

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

Closed
bholley opened this issue Jan 26, 2018 · 5 comments

Comments

@bholley
Copy link
Contributor

bholley commented Jan 26, 2018

I've been working on removing Debug implementations from Firefox release builds to improve code size. One common culprit is assert_eq!(), which generally takes integers, but formats them with {:?}.

I first assumed that this would probably call straight into Display, or at the very least instantiate a single adapter. Unfortunately, we appear to end up with one adapter per callsite, because the adapters use relative jumps.

So we end up with tons of functions of the form: https://gist.github.com/bholley/b3b8b3a9df649334263338955c10f63d

Which are all identical, except for the offset in the jump instruction: https://gist.github.com/bholley/e2c4ceec5cc3bc38c2462f7cc4bfd242

I'm not sure if this is an LLVM bug or a rustc bug, but it sure seems sub-optimal.

CC @SimonSapin @nox @Manishearth @emilio

@kennytm
Copy link
Member

kennytm commented Jan 26, 2018

I think this is related to https://internals.rust-lang.org/t/changing-core-fmt-for-speed/5154.

@bluss
Copy link
Member

bluss commented Jan 26, 2018

Have you managed to reproduce this in a relatively small program? How do we make an example where the adapter shows up more than once?

@bholley
Copy link
Contributor Author

bholley commented Jan 26, 2018

So I tried reproducing with [1], but it didn't work. I then had another look at the binary I've been analyzing, and it appears that I was mistaken about there being an adapter for every callsite. There are many adapters, but they seem to be spread through the code, at intervals of around 64k. Each instance is only referenced by code near to it, generally with an LEA.

So my guess is that LLVM is doing this to allow relative address loads with 16-bit operands. It still seems like we should avoid the adapters entirely, but that's probably better addressed in the issue that @kennytm mentioned.

So I'll go ahead and close this hear. Sorry for the noise.

[1] https://play.rust-lang.org/?gist=fbd0b57c6e86763072dd148bb629f7ce&version=stable

@bholley bholley closed this as completed Jan 26, 2018
@bluss
Copy link
Member

bluss commented Jan 26, 2018

It could be beneficial to use #[inline] on the Debug impls for all the integer types -- since they just call the Display fmt?

@bholley
Copy link
Contributor Author

bholley commented Jan 26, 2018

That does seem worth doing, assuming it's not too hard.

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

No branches or pull requests

3 participants