Skip to content

Sub-optimal layout of Option<(u16, [u8; 15])> (missed niche optimization) #117429

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

Open
gendx opened this issue Oct 31, 2023 · 3 comments
Open

Sub-optimal layout of Option<(u16, [u8; 15])> (missed niche optimization) #117429

gendx opened this issue Oct 31, 2023 · 3 comments
Labels
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

@gendx
Copy link

gendx commented Oct 31, 2023

I tried this code:

use std::mem::size_of;

fn main() {
    println!("(u16, [u8; 14]) = {} bytes", size_of::<(u16, [u8; 14])>());
    println!("(u16, [u8; 15]) = {} bytes", size_of::<(u16, [u8; 15])>());
    
    println!("Option<(u16, [u8; 14])> = {} bytes", size_of::<Option<(u16, [u8; 14])>>());
    println!("Option<(u16, [u8; 15])> = {} bytes", size_of::<Option<(u16, [u8; 15])>>());
    
    let mut x: Option<(u16, [u8; 15])> = None;
    let bytes: [u8; 20] = unsafe { std::mem::transmute(x) };
    println!("None = {:02x?}", bytes);

    x = Some((0xffff, [0xee; 15]));
    let bytes: [u8; 20] = unsafe { std::mem::transmute(x) };
    println!("Some = {:02x?}", bytes);
}

I expected to see this happen:

  • (u16, [u8; 15]) has a size of 18 bytes, because there are 17 bytes of useful data and it is rounded up due to the alignment of 2 bytes.
  • I expected Option<(u16, [u8; 15])> to also take 18 bytes due to a niche optimization.

Instead, this happened:

  • Option<(u16, [u8; 15])> takes 20 bytes, and appears to be layout as:
struct Foo {
    discriminant: u16,
    x0: u16,
    x1: [u8; 15],
    padding: u8,
}

I expected the Option to leverage the byte of padding at the end of the struct to decide if it's None or Some instead of pre-pending a discriminant.

(u16, [u8; 14]) = 16 bytes
(u16, [u8; 15]) = 18 bytes
Option<(u16, [u8; 14])> = 18 bytes
Option<(u16, [u8; 15])> = 20 bytes
None = [00, 00, 00, 00, 54, d1, 5c, fc, b6, 7f, 00, 00, 05, 00, 00, 00, 00, 00, 00, 00]
Some = [01, 00, ff, ff, ee, ee, ee, ee, ee, ee, ee, ee, ee, ee, ee, ee, ee, ee, ee, f7]

Meta

Tested on the playground: https://play.rust-lang.org/?version=stable&mode=release&edition=2021


This is inspired by the tinyvec crate, which is roughly layout as follows (https://docs.rs/tinyvec/latest/src/tinyvec/tinyvec.rs.html#97-103, https://docs.rs/tinyvec/latest/src/tinyvec/arrayvec.rs.html#103-107) - although the niche wouldn't apply in that case (and the smallvec crate has a better layout):

pub enum TinyVec<T, const N: usize> {
    Inline((u16, [T; N])),
    Heap(Vec<T>),
}
@gendx gendx added the C-bug Category: This is a bug. label Oct 31, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 31, 2023
@Noratrieb Noratrieb added 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. A-layout Area: Memory layout of types and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 31, 2023
@cuviper
Copy link
Member

cuviper commented Nov 10, 2023

Implicit padding is not a niche, because any bits are valid there in memory -- e.g. for Option, there's no value that we could read in the padding and say that this must be a None. (even setting aside whether padding bytes are undef)

Another way to think about this: Given an Option<Foo(u16, u8)> that is Some, you can get &mut Foo and pass that to arbitrary code that has no idea about the original Option layout. Any Foo value can be written there that clobbers the padding byte as well, writing the full size_of::<Foo>().

@cuviper
Copy link
Member

cuviper commented Nov 10, 2023

See also #70230.

@ryanavella
Copy link

I have a crate struct-pad that I wrote for exactly this purpose. The implementation is simple enough that you could write it yourself.

Changing your Option<(u16, [u8; 15])> to Option<(u16, [u8; 15], PadU8)> shrinks the size from 20 bytes to 18 bytes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

No branches or pull requests

5 participants