Skip to content
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

Missed niche optimization (Box<impl Sized> & Box<impl Unsized>) #66029

Closed
CAD97 opened this issue Nov 1, 2019 · 5 comments
Closed

Missed niche optimization (Box<impl Sized> & Box<impl Unsized>) #66029

CAD97 opened this issue Nov 1, 2019 · 5 comments
Labels
A-codegen Area: Code generation A-layout Area: Memory layout of types C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@CAD97
Copy link
Contributor

CAD97 commented Nov 1, 2019

playground

enum WithBox {
    Sized(Box<Sized>),
    Unsized(Box<Unsized>),
}

enum WithUnitPtr {
    Sized(  /*   0usize    */ ptr::NonNull<()>),
    Unsized(ptr::NonNull<()>, usize           ),
}

fn main() {
    dbg!(size_of::<WithBox>());        // = 24
    dbg!(size_of::<WithUnitPtr>());    // = 24
    dbg!(size_of::<ManuallyNiched>()); // = 16
}

struct ManuallyNiched {
    a: usize,
    b: usize,
}

impl From<WithBox> for ManuallyNiched {
    fn from(this: WithBox) -> ManuallyNiched {
        match this {
            WithBox::Sized(this) => ManuallyNiched {
                a: 0,
                b: Box::into_raw(this) as usize,
            },
            WithBox::Unsized(this) => ManuallyNiched {
                b: this.len(),
                a: Box::into_raw(this) as *mut Sized as usize,
            },
        }
    }
}

impl From<ManuallyNiched> for WithBox {
    fn from(this: ManuallyNiched) -> WithBox {
        unsafe {
            match this.a {
                0 => WithBox::Sized(Box::from_raw(this.b as *mut _)),
                _ => WithBox::Unsized(Box::from_raw(slice::from_raw_parts_mut(
                    this.a as *mut _,
                    this.b,
                ))),
            }
        }
    }
}

Specifically, the tagged union of ptr::NonNull<{Sized type}> and ptr::NonNull<{!Sized type}> can niche together to be the size of ptr::NonNull<{!Sized type}> (for unsized types with non-zero sized metadata) by storing the discriminant for the sized pointer in the 0 niche of the non-null pointer to the unsized type (and the pointer to the sized type in the pointer metadata of the unsized type).

@moshg
Copy link

moshg commented Nov 3, 2019

I think this is a special case of #46213.

@Alexendoo Alexendoo added A-codegen Area: Code generation C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Nov 6, 2019
@petertodd
Copy link
Contributor

Maybe I'm misunderstanding what you're trying to demonstrate here, but the type:

type Unsized = [usize];

doesn't lead to Box<[usize]> niche other than the NonNull pointer, as a slice's metadata can have any value, including zero¹.

Changing that to, say:

type Unsized = dyn Drop;

is a better example, as the vtable should never be null.

  1. ...actually a slice length should be < isize::MAX. But IIUC the current niche scheme can't handle niches where the valid range ends before a certain point; it only handles cases where the valid range starts at a certain point.

@CAD97
Copy link
Contributor Author

CAD97 commented Nov 8, 2019

@petertodd that one niche is enough. Box<Sized> is one usize big, and Box<Unsized> is two usize big. The niched representation is to represent Box<Unsized> as is, and to represent Box<Sized> by exploiting the 0 niche in the nonnull pointer of Box<Unsized> and then putting the Box<Sized> in the other remaining usize of space.

@petertodd
Copy link
Contributor

@CAD97 Thanks! I see what you mean now.

@Alexendoo Alexendoo added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 26, 2020
@jonas-schievink jonas-schievink added the A-layout Area: Memory layout of types label Mar 29, 2020
@CAD97
Copy link
Contributor Author

CAD97 commented Jul 30, 2023

This niche is now used and the playground prints 16 (on 64-bit) for the tested enums. Going ahead and closing as completed, as I don't think this needs a regression test (it's covered by other niche tests and isn't a guaranteed behavior).

@CAD97 CAD97 closed this as completed Jul 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-layout Area: Memory layout of types C-enhancement Category: An issue proposing an enhancement or a PR with one. 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.

5 participants