Fix float bug in reserve_for_insert#348
Open
kraldan wants to merge 1 commit intoboostorg:developfrom
Open
Conversation
The same fix as in https://github.com/boostorg/multi_index/pull/94/changes
Member
|
Can you please update your branch to add a test that fails without this change in one commit and then fix in the following commit? You can run b2 easily from the boost root just using |
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.
Intro
Hi, I found practically the same bug as in boostorg/multi_index#94.
reserve_for_insertuses floats to calculate the next number of buckets. These can round down and not increase the bucket count, which leads the table being rehashed on multiple subsequent inserts that rely on this method.The following program demonstrates this.
Code
(I'm sorry for it being memory-heavy, it was the simplest way for me to demonstrate.)
On my machine, with the current implementation, the output is:
container size: 98318, insert time: 1 ms
container size: 196614, insert time: 2 ms
container size: 393242, insert time: 5 ms
container size: 786434, insert time: 11 ms
container size: 1572870, insert time: 22 ms
container size: 3145740, insert time: 46 ms
container size: 6291470, insert time: 101 ms
container size: 12582918, insert time: 180 ms
container size: 25165844, insert time: 355 ms
container size: 50331654, insert time: 729 ms
container size: 100663320, insert time: 1686 ms
container size: 201326612, insert time: 2113 ms
container size: 201326613, insert time: 2138 ms
container size: 201326614, insert time: 2259 ms
container size: 201326615, insert time: 2121 ms
container size: 201326616, insert time: 3139 ms
container size: 402653190, insert time: 5819 ms
container size: 402653191, insert time: 4343 ms
container size: 402653192, insert time: 4516 ms
container size: 402653193, insert time: 4377 ms
container size: 402653194, insert time: 4263 ms
container size: 402653195, insert time: 4329 ms
container size: 402653196, insert time: 4225 ms
container size: 402653197, insert time: 4229 ms
container size: 402653198, insert time: 4220 ms
container size: 402653199, insert time: 4322 ms
container size: 402653200, insert time: 4181 ms
container size: 402653201, insert time: 9137 ms
On my machine, with the the fix, the output is:
container size: 98318, insert time: 1 ms
container size: 196614, insert time: 2 ms
container size: 393242, insert time: 5 ms
container size: 786434, insert time: 11 ms
container size: 1572870, insert time: 23 ms
container size: 3145740, insert time: 48 ms
container size: 6291470, insert time: 93 ms
container size: 12582918, insert time: 183 ms
container size: 25165844, insert time: 363 ms
container size: 50331654, insert time: 719 ms
container size: 100663320, insert time: 1476 ms
container size: 201326612, insert time: 2947 ms
container size: 402653190, insert time: 7719 ms
The fix
I implemented the fix to mirror https://github.com/boostorg/multi_index/pull/94/changes