[WIP] Use a union to reduce the size of SmallVec#92
Closed
Amanieu wants to merge 2 commits intoservo:masterfrom
Closed
[WIP] Use a union to reduce the size of SmallVec#92Amanieu wants to merge 2 commits intoservo:masterfrom
Amanieu wants to merge 2 commits intoservo:masterfrom
Conversation
Contributor
Author
|
Hmm there seems to be a regression in the benchmark results. I will look into this. |
emilio
reviewed
Apr 17, 2018
| unsafe fn heap(&self) -> (*mut A::Item, usize) { | ||
| match *self { | ||
| SmallVecData::Heap(data) => data, | ||
| _ => unreachable!(), |
Member
There was a problem hiding this comment.
debug_unreachable may be worth in these.
Contributor
|
☔ The latest upstream changes (presumably #93) made this pull request unmergeable. Please resolve the merge conflicts. |
Contributor
|
You might want to recommit this without rustfmt. Ideally, the rustfmt would be a seperate PR. Also, is the change from an |
Contributor
Author
|
I'm going to close this since I haven't had time to work on it lately. |
bors-servo
pushed a commit
that referenced
this pull request
Jun 6, 2018
Use a union to reduce the size of SmallVec [v2] Building on top of #92 by @Amanieu I introduced `triple()` and `triple_mut()` which removed almost all of the runtime overhead. Performance is very comparable. ``` name master:: ns/iter union:: ns/iter diff ns/iter diff % speedup bench_extend 45 47 2 4.44% x 0.96 bench_extend_from_slice 45 43 -2 -4.44% x 1.05 bench_from_slice 45 44 -1 -2.22% x 1.02 bench_insert 615 622 7 1.14% x 0.99 bench_insert_from_slice 101 99 -2 -1.98% x 1.02 bench_insert_many 309 266 -43 -13.92% x 1.16 bench_macro_from_elem 41 37 -4 -9.76% x 1.11 bench_macro_from_list 40 42 2 5.00% x 0.95 bench_push 381 370 -11 -2.89% x 1.03 bench_pushpop 404 420 16 3.96% x 0.96 bench_remove 458 436 -22 -4.80% x 1.05 ``` <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/94) <!-- Reviewable:end -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uses a union to eliminate the space used by the
enumdeterminant. The new layout looks like this:The
capacityfield is used to determine which of the two union variants is active:capacity <= A::size()then the inline variant is used andcapacityholds the current length of the vector (number of elements actually in use).capacity > A::size()then the heap variant is used andcapacityholds the size of the memory allocation.Since unions with drop are still currently unstable, this is kept behind a "union" cargo feature, which falls back to an implementation using an enum if disabled.
This change is