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 optimization when putting structs with padding an enum #109555

Closed
HomelikeBrick42 opened this issue Mar 24, 2023 · 4 comments
Closed

Missed optimization when putting structs with padding an enum #109555

HomelikeBrick42 opened this issue Mar 24, 2023 · 4 comments

Comments

@HomelikeBrick42
Copy link

HomelikeBrick42 commented Mar 24, 2023

struct S {
    a: u32,
    b: i128,
}

enum Foo {
    Var1(S),
    Var2(S),
}

use std::mem::size_of;
dbg!(size_of::<S>());
dbg!(size_of::<Foo>());

this code prints

[src/main.rs:13] size_of::<S>() = 24
[src/main.rs:14] size_of::<Foo>() = 32

when it could have instead used the free padding bytes in S for the discriminant, so Foo would be the same size as S

@SkiFire13
Copy link
Contributor

This is incompatible with the assumption that you can write a value of type S by copying std::mem::size_of::<S>() bytes, because that would overwrite the discriminant with uninit bytes.

@cuviper
Copy link
Member

cuviper commented Mar 24, 2023

See also #70230 to define padding such that niches are possible. However, I think that still wouldn't allow Foo to work as you want, because we only write tags in a niche when it is not that variant, and no other variants overlap that niche.

@the8472
Copy link
Member

the8472 commented Mar 24, 2023

Today you can use repr(packed) struct S {...} to shrink the struct. The hypothetical stride != size layout would also enable that optimization.

@scottmcm
Copy link
Member

I'm going to close this because it's not currently legal to do such an optimization because of the semantics of padding. This would need a language change (like move-only fields, or different padding semantics) to be allowed, but language changes are for IRLO or zulip, not this issue tracker.

I'll note that you can get layout optimizations in certain situations by adding a restriction instead of having padding:

#[repr(u8)]
enum ZeroByte { Zero = 0 }

struct S {
    a: u32,
    b: i128,
}

struct S2 {
    a: u32,
    b: i128,
    c: ZeroByte,
}

fn main() {
    use std::mem::size_of;
    dbg!(size_of::<S>());
    dbg!(size_of::<Option<S>>());
    dbg!(size_of::<S2>());
    dbg!(size_of::<Option<S2>>());
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9df3eac94015ea2d499903c1daf475ce

[src/main.rs:19] size_of::<S>() = 24
[src/main.rs:20] size_of::<Option<S>>() = 32
[src/main.rs:21] size_of::<S2>() = 24
[src/main.rs:22] size_of::<Option<S2>>() = 24

@scottmcm scottmcm closed this as not planned Won't fix, can't repro, duplicate, stale Mar 25, 2023
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

5 participants