Fixed lockfree frozen vec being too small. #81
Conversation
|
r? @oli-obk |
| const NUM_BUFFERS: usize = (usize::BITS >> 2) as usize; | ||
| /// Each buffer covers 2 bits of address space, so we need half as many | ||
| /// buffers as bits in a usize. | ||
| const NUM_BUFFERS: usize = (usize::BITS / 2) as usize; |
There was a problem hiding this comment.
Good catch. I had an off by one here in my head because I always calculated the total buffer size including the buffer at index NUM_BUFFERS.
Maybe we should grow buffers faster instead of increasing the number of buffers?
There was a problem hiding this comment.
The buffers are already growing at 4x each time it grows, which is quite aggresive. Not increasing the buffer size would require an 8x growth factor, which just seems excessive to me. Especially given that a few non-breaking versions ago (1.7.1/1.8.1) this was only growing 2x at each cutover point.
There was a problem hiding this comment.
Note that this also means that in some cases 8 times more storage would be used than is strictly required to store the elements. I think that that is not worth the small reduction in number of pointers used.
davidv1992
force-pushed
the
lockfreefrozenvec-too-small
branch
from
1db9f2f to
9c75ab5
Compare
|
I'll make a release with this and #82 when I get a chance |
|
awesome, thanks for handling this as quickly as you have. |
|
Published |
The current implementation accidently divides by 4 instead of 2 when calculating the number of buffers required. This means that on 32 bit systems only 65535 items can be in a
LockFreeFrozenVec, which the rust compiler exceeds. (See also https://github.com/rust-lang-ci/rust/actions/runs/12980407836/job/36197397174, triggered by rust-lang/rust#136094)Is it possible to release a new version after this bugfix has been merged?