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

Enum layout optimizations #19536

Closed
wants to merge 11 commits into from
Closed

Conversation

luqmana
Copy link
Member

@luqmana luqmana commented Dec 4, 2014

Part 1: This extends the nullable enum opt to traverse beyond just the first level to find possible fields to use as the discriminant. So now, it'll work through structs, tuples, and fixed sized arrays.

Part 2: Introduce a new lang item, NonZero, that you can use to wrap raw pointers or integral types to indicate to rustc that the underlying value is known to never be 0/NULL. We then use this in Vec, Rc and Arc to have them also benefit from the nullable enum opt.

x86_64 Linux:
                        T       Option<T> (Before)      Option<T> (After)
----------------------------------------------------------------------------------
Vec<int>                24          32                      24
String                  24          32                      24
Rc<int>                 8           16                      8
Arc<int>                8           16                      8
[Box<int>, ..2]         16          24                      16
(String, uint)          32          40                      32

Fixes #19419.
Fixes #13194.
Fixes #9378.
Fixes #7576.

@sfackler
Copy link
Member

sfackler commented Dec 4, 2014

Tidy error

@luqmana luqmana force-pushed the nonzero-lang-item branch 4 times, most recently from 9b6b89b to fcd88dd Compare December 4, 2014 22:15
@lifthrasiir
Copy link
Contributor

Great! By the way, if my understanding is correct, you cannot (yet) optimize Option<Option<[Box<T>, ..2]>> into two words but this can be changed via find_discr_field_candidate, right?

@Gankra
Copy link
Contributor

Gankra commented Dec 5, 2014

CC @aturon on NonZero

@luqmana
Copy link
Member Author

luqmana commented Dec 5, 2014

@lifthrasiir yep, though i'd imagine sharing discriminants with nested enums will require a bit of work.

@ftxqxd
Copy link
Contributor

ftxqxd commented Dec 5, 2014

There has already been an RFC for extending the nullable pointer optimisation to library types, but it was closed & postponed. It does have a slightly different design to this pull request’s, though: it used an attribute instead of a wrapper type, and didn’t allow arbitrary types to be non-nullable—only raw pointers were allowed.

@alexcrichton
Copy link
Member

@luqmana For now would you be ok leaving out NonZero? @P1start brings up a good point in that we've current postponed this modification, and we should be able to add it backwards compatibly in the future.

@alexcrichton
Copy link
Member

These are some awesome improvements by the way, nice work! Could this also add some tests using mem::size_of to ensure we don't regress on this optimization?

@luqmana luqmana force-pushed the nonzero-lang-item branch 2 times, most recently from 218b772 to 0d7d9f7 Compare December 5, 2014 19:45
@reem
Copy link
Contributor

reem commented Dec 6, 2014

@alexcrichton It looks like most of the gains here come from the use of NonZero - without it Option<Vec<T>> would still be 32 not 24, meaning we actually wouldn't get as much benefit.

@alexcrichton
Copy link
Member

@luqmana do you want to wait for rust-lang/rfcs#499 to land this? If so, would you be ok closing in favor of the RFC?

@luqmana
Copy link
Member Author

luqmana commented Dec 8, 2014

@alexcrichton sure that's fine
On Dec 8, 2014 2:43 PM, "Alex Crichton" notifications@github.com wrote:

@luqmana https://github.com/luqmana do you want to wait for
rust-lang/rfcs#499 rust-lang/rfcs#499 to land
this? If so, would you be ok closing in favor of the RFC?


Reply to this email directly or view it on GitHub
#19536 (comment).

@luqmana luqmana closed this Dec 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants