Skip to content

Niche-filling layouts are picked over tagged ones even when detrimental. #63866

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
eddyb opened this issue Aug 24, 2019 · 0 comments · Fixed by #74069
Closed

Niche-filling layouts are picked over tagged ones even when detrimental. #63866

eddyb opened this issue Aug 24, 2019 · 0 comments · Fixed by #74069
Labels
A-layout Area: Memory layout of types C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Aug 24, 2019

This prints the values in comments:

use std::num::NonZeroU8;

pub enum Foo { A(NonZeroU8, u32), B }
pub enum Bar { A(u8, u32), B }

fn main() {
    use std::mem::size_of;
    
    dbg!(size_of::<Foo>()); // = 8
    dbg!(size_of::<Option<Foo>>()); // = 12
    dbg!(size_of::<Option<Option<Foo>>>()); // = 12
    
    dbg!(size_of::<Bar>()); // = 8
    dbg!(size_of::<Option<Bar>>()); // = 8
    dbg!(size_of::<Option<Option<Bar>>>()); // = 8
}

Note that Foo contains strictly more opportunities for optimization, but it's optimized worse.

That's because it uses a "niche-filling" layout (using 0 in the NonZeroU8 field of A for B), but the "tag" layout wouldn't be any larger, because it has enough space in 8 bytes to fit the (byte-sized) tag, NonZeroU8 and u32.

Bar uses the tagged layout since it has no other choice, and ends up with 254 free values in the byte-sized tag, but Foo "jumps the gun" on using a pre-existing niche.

@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 24, 2019
@jonas-schievink jonas-schievink added the A-layout Area: Memory layout of types label Mar 29, 2020
ogoffart added a commit to ogoffart/rust that referenced this issue Apr 16, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 18, 2020
…sakis

Compare tagged/niche-filling layout and pick the best one

Finishes up rust-lang#71045, and so fixes rust-lang#63866.

cc @eddyb
r? @nikomatsakis (since @eddyb wrote the first commit)
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 18, 2020
…sakis

Compare tagged/niche-filling layout and pick the best one

Finishes up rust-lang#71045, and so fixes rust-lang#63866.

cc @eddyb
r? @nikomatsakis (since @eddyb wrote the first commit)
@bors bors closed this as completed in e775b4d Jul 18, 2020
rust-timer added a commit to rust-timer/rust that referenced this issue Jul 21, 2020
Original message:
Rollup merge of rust-lang#74069 - erikdesjardins:bad-niche, r=nikomatsakis

Compare tagged/niche-filling layout and pick the best one

Finishes up rust-lang#71045, and so fixes rust-lang#63866.

cc @eddyb
r? @nikomatsakis (since @eddyb wrote the first commit)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-layout Area: Memory layout of types C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants