Avoid extra usize array by unioning next pointer into elements #1
Conversation
This does not yet realize the space savings it promises in the test case due to missing optimizations[1][2] in Rust, but should eventually manage to be barely larger than the payload arrays. [1]: rust-lang/rust#46213 [2]: rust-lang/rust#70477
|
I'm not comfortable with having failing tests. Please comment it out as it will cause unnecessary noise in the build results. Other than that, the idea is interesting but since I'm new to Rust, I'll need some time to accept this. Thanks for your contribution. |
|
The test was meant to stop easy merging as it doesn't give tangible benefits yet. If you think that it's a valuable contribution still, I'm happy to adjust the test. (I don't know the test system by heart yet; in Python I'd annotate it with "expect failure", here I'll need to look through the docs to see how to best annotate it before falling back to commenting it out.) |
|
Ah sorry I submitted the review without checking your response. I'm fine with merging this early. |
|
It's possible to mark tests with |
Of the two versions (one previously commented out), picking the one that can be read more straight-forward. Test cases needed adapting because they made assumptions on the allocation sequence not warranted by the current implementation. See-Also: bugadani#1 (comment)
Codecov Report
@@ Coverage Diff @@
## master #1 +/- ##
==========================================
- Coverage 67.91% 60.13% -7.78%
==========================================
Files 2 2
Lines 134 148 +14
Branches 16 21 +5
==========================================
- Hits 91 89 -2
- Misses 18 28 +10
- Partials 25 31 +6
Continue to review full report at Codecov.
|
|
I've seen that there's a few warnings still in the tests, and reduced code coverage; taking another look at it. |
As those are not evaluated in the allocation call tree any more, they previously had lost their test coverage.
3 participants
This does not yet realize the space savings it promises in the test case
due to missing optimizations (1, 2) in Rust, but should eventually manage
to be barely larger than the payload arrays.
The current code has parallel arrays for used and free slots, where actually only ever one or the other is ever used. By putting them in a single enum, a slot is either used for the free list or for data.
Ideally, this would do away with the need for per-element data completely, provided that the elements have a niche, and a usize fits next to unique bits of that niche. Unfortunately, that optimization is not present in Rust yet (see the above linked issue / PR), thus there are no actual savings from this PR yet.
There is some further potential for optimization in this, in particular:
Instead of a usize, the minimal type required for the given array size could be used.
That would allow the use with 16bit niched types and an u8 index without any additional space even on long-addess architectures.
Collaps EmptyNext and EmptyLast using knowledge of the free counter, allowing optimization where IT has exactly one niche and then only indexed-size bytes left.
The last element would then have EmptyNext(anything) in it, but it wouldn't matter because free says that there's nothing left.
Reduce the constant overhead by utilizing the "free" counter as an indicator whether next_free is valid.