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

Niche in Arc<..> variant of enum not used by Option<..> #125363

Open
horazont opened this issue May 21, 2024 · 2 comments
Open

Niche in Arc<..> variant of enum not used by Option<..> #125363

horazont opened this issue May 21, 2024 · 2 comments
Labels
A-layout Area: Memory layout of types C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@horazont
Copy link

horazont commented May 21, 2024

Consider the following code (playground):

#![allow(dead_code)]
use std::mem::size_of;
use std::sync::Arc;

enum Foo {
  A(&'static str),
  B(Arc<()>),
}

enum Bar {
  A(&'static str),
  B(Option<Arc<()>>),
}

fn main() {
    println!("sizes of Foo: {} w/ Option {}", size_of::<Foo>(), size_of::<Option<Foo>>());
    print!("repr of Foo::A(..):            ");
    dump_repr(Foo::A("hello"));
    print!("repr of Foo::B(..):            ");
    dump_repr(Foo::B(Arc::new(())));
    print!("repr of Option<Foo>::None:     ");
    dump_repr(Option::<Foo>::None);
    print!("repr of Option<Foo>::Some(..): ");
    dump_repr(Option::<Foo>::Some(Foo::A("hello")));
    println!("sizes of Bar: {} w/ Option {}", size_of::<Bar>(), size_of::<Option<Bar>>());
    print!("repr of Bar::A(..):            ");
    dump_repr(Bar::A("hello"));
    print!("repr of Bar::B(Some(..)):      ");
    dump_repr(Bar::B(Some(Arc::new(()))));
    print!("repr of Bar::B(None):          ");
    dump_repr(Bar::B(None));
}

fn dump_repr<T>(a: T) {
    let sz = size_of::<T>();
    let usize_sz = size_of::<usize>();
    assert_eq!(sz % usize_sz, 0);
    let n_usize = sz / usize_sz;
    let b = &a as *const T as *const usize;
    let b = unsafe { std::slice::from_raw_parts(b, n_usize) };
    for word in b {
        print!("{:016x} ", word);
    }
    println!();
}

The Foo enum contains two niches:

  • The slice in Foo::A(&str) has a non-nullable field.
  • The Arc in Foo::B(Arc<_>) has a non-nullable field.

The compiler correctly places the distinction between the variants A and B in one of these fields, concretely in the non-nullable field of the slice. The pointer for the Arc is then placed in the second half of the memory, which is otherwise the slice's length.

Unfortunately, wrapping Foo in Option<_> does not make use of the remaining niche: It could set both niches to null (basically selecting the Foo::B variant and nulling the Arc's NotNull pointer) to indicate the None variant.

Instead, we get a separate discriminant field, as can be seen by running the above example.

Note that the niche in Arc is not used at all by Foo. This can be seen in the Bar version of the enum, whose size does not increase by changing the Arc<_> into an Option<Arc<_>>.

I honestly have no idea about how niche tracking works and how much work it is to implement this, but I thought I'd leave this here as a kind of inspiration or tracking issue.

I note that there are a couple issues around this, but I think that this is not a duplicate of:

Thank you for reading.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 21, 2024
@the8472
Copy link
Member

the8472 commented May 21, 2024

Unfortunately, wrapping Foo in Option<_> does not make use of the remaining niche: It could set both niches to null (basically selecting the Foo::B variant and nulling the Arc's NotNull pointer) to indicate the None variant.

That would require two-usize comparison (or arbitrarily-wide comparison if this scheme were extended to more fields) which is more expensive than single-register operation.

#119507 could be related, but it's about structs. Not sure if that qualifies as dup.

Enum variant payload and struct layout share the same code. So yes, it's quite similar. Though that issue talks about 2x NonZeroU8, which could still be merged into a single register.

@the8472 the8472 added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-layout Area: Memory layout of types C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 21, 2024
@horazont
Copy link
Author

Unfortunately, wrapping Foo in Option<_> does not make use of the remaining niche: It could set both niches to null (basically selecting the Foo::B variant and nulling the Arc's NotNull pointer) to indicate the None variant.

That would require two-usize comparison (or arbitrarily-wide comparison if this scheme were extended to more fields) which is more expensive than single-register operation.

Aha, indeed. I had noticed that two reads are needed for the Some(_) variants, which is the same in the non-inlined case. I did not see that two reads would then also needed for the None variant, which is of course a drawback.

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-optimization Category: An issue highlighting optimization opportunities or PRs implementing such 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

3 participants