-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
perf(query): Update CompressedBin IntersectionAlgo #9000
Merged
Merged
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
20a7a77
updated test
4e458f2
Fixed CompressedBin algo, updated heuristic based on benchmarks
a0b1e23
updated algo
0945878
updated algo
b8b96d6
Fixed tests
7bf9605
fixed test
d350c12
Fixed failing tests, updated existing test to be more robust
36b2722
Fixed comment
170f37c
Fixed comments
f1e65ae
Fixed failing test
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -223,6 +223,64 @@ func (d *Decoder) ApproxLen() int { | |
|
||
type searchFunc func(int) bool | ||
|
||
// SeekToBlock will find the nearest block, and unpack it. Unlike Seek, it doesn't | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can improve this explanation |
||
// apply search in the resulting uid list and then move the pointer forward. When we are going | ||
// to intersect the list later, this function is useful. | ||
func (d *Decoder) SeekToBlock(uid uint64, whence seekPos) []uint64 { | ||
if d.Pack == nil { | ||
return []uint64{} | ||
} | ||
prevBlockIdx := d.blockIdx | ||
d.blockIdx = 0 | ||
if uid == 0 { | ||
return d.UnpackBlock() | ||
} | ||
|
||
// If for some reason we are searching an older uid, we need to search the entire pack | ||
if prevBlockIdx > 0 && uid < d.Pack.Blocks[prevBlockIdx].Base { | ||
prevBlockIdx = 0 | ||
} | ||
|
||
pack := d.Pack | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove this assignment |
||
blocksFunc := func() searchFunc { | ||
var f searchFunc | ||
switch whence { | ||
case SeekStart: | ||
f = func(i int) bool { return pack.Blocks[i+prevBlockIdx].Base >= uid } | ||
case SeekCurrent: | ||
f = func(i int) bool { return pack.Blocks[i+prevBlockIdx].Base > uid } | ||
} | ||
return f | ||
} | ||
|
||
idx := sort.Search(len(pack.Blocks[prevBlockIdx:]), blocksFunc()) + prevBlockIdx | ||
// The first block.Base >= uid. | ||
if idx == 0 { | ||
return d.UnpackBlock() | ||
} | ||
// The uid is the first entry in the block. | ||
if idx < len(pack.Blocks) && pack.Blocks[idx].Base == uid { | ||
d.blockIdx = idx | ||
return d.UnpackBlock() | ||
} | ||
|
||
// Either the idx = len(pack.Blocks) that means it wasn't found in any of the block's base. Or, | ||
// we found the first block index whose base is greater than uid. In these cases, go to the | ||
// previous block and search there. | ||
d.blockIdx = idx - 1 // Move to the previous block. If blockIdx<0, unpack will deal with it. | ||
if d.blockIdx != prevBlockIdx { | ||
d.UnpackBlock() // And get all their uids. | ||
} | ||
|
||
if uid <= d.uids[len(d.uids)-1] { | ||
return d.uids | ||
} | ||
|
||
// Could not find any uid in the block, which is >= uid. The next block might still have valid | ||
// entries > uid. | ||
return d.Next() | ||
} | ||
|
||
// Seek will search for uid in a packed block using the specified whence position. | ||
// The value of whence must be one of the predefined values SeekStart or SeekCurrent. | ||
// SeekStart searches uid and includes it as part of the results. | ||
|
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.
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.
fix this condition
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.
I tried updating the condition, but it made performance much worse. I am guessing its because that the decision to do binary or linear really depends on the total size of the arrays. If the array ratio is too high, then the numbers would be far apart.