-
Notifications
You must be signed in to change notification settings - Fork 1.2k
BVSparse: keep 'last found' for fast fromIndex discovery #4588
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
Conversation
| BVSparseNode<TAllocator> * | ||
| BVSparse<TAllocator>::DeleteNode(BVSparseNode *node, bool bResetLastUsed) | ||
| { | ||
| this->tail = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we necessarily only delete the last node? If so, shouldn't we update the tail to be the new last node?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated the variable name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that clears things up a lot, thanks!
e95afaf to
8692a9d
Compare
8692a9d to
136a7b4
Compare
Penguinwizzard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@Penguinwizzard Thanks for the review! |
…ex discovery Merge pull request #4588 from obastemur:faster_bvsparse Helps the number of tests not to timeout on CI machines.
…st fromIndex discovery Merge pull request #4588 from obastemur:faster_bvsparse Helps the number of tests not to timeout on CI machines.
…found' for fast fromIndex discovery Merge pull request #4588 from obastemur:faster_bvsparse Helps the number of tests not to timeout on CI machines.
Helps the number of tests not to timeout on CI machines.