-
Notifications
You must be signed in to change notification settings - Fork 177
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
Add direct
indices to datastore
#2221
Conversation
This implementation uses `Option<Box<[RowPointer; KEYS_PER_INNER]>>` to represent inner indices. This reduces the space used by the outer index from 24 to 8 bytes, making randomized inserts -73% faster, but still slow. Monotonic inserts also get -20% faster.
ce9f6a6
to
b95f936
Compare
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 have completely ignored your benchmarks during this review; I honestly don't care about them. Also, given our current time constraints, I haven't given this as careful of a review as I would like. But it all looks reasonable, and the concept here is so dirt-simple that it seems hard to do wrong, honestly.
Description of Changes
The PR is best reviewed commit by commit, as they build upon the previous one and do one thing.
Adds a note re. renaming
BTreeIndex
-- later fixed in 6-7.Adds
direct
indices to the datastore.Adds micro-benchmarks for some known index implementations, including our unique btrees and direct indices.
Renames
BTreeIndex::seek
toBTreeIndex::seek_range
and consequence changes.Adds
BTreeIndex::seek_point
, which makes point scans about 12% faster in micro-benchmarks. This is then employed in the various places where we currently just use&AlgebraicValue
as a bound. More can be done here however in follow ups.6-7. Rename
BTreeIndex
toTableIndex
to reflect that it isn't limited to btrees anymore.bindings-macro
, necessary to actually use#[index(direct)]
.With
#[index(direct)]
applied to tablesPosition
andVelocity
in the update benchmark, we get an average 357ms, which is in line with what we found when we hacked in the index implementation.This also improved subscription benchmarks as compared to current master (
footprint-semijoin
: -22%):API and ABI breaking changes
None.
Expected complexity level and risk
3, the PR is large, but broken into individual commits that aren't that complicated.
There's also some
unsafe
in the index implementation.Testing
Existing tests are amended and some new unit tests are added in the commit that adds
direct
indices.The benchmark has also been run, exercising
#[index(direct)]
in a module.