Add optimized implementation for string indexes #632
Merged
Conversation
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
|
I haven't had the time yet, but it would be great to have some numbers on the performance delta. Ideally for both space and time. |
|
There's a fundamental issue I overlooked in the design: the index does not support inequality lookups. It can't because it hashes the input. We'll have to reconsider the usefulness of a space-efficient equality-only string index before proceeding. |
mavam
force-pushed
the
story/ch4493
branch
3 times, most recently
from
ce68bfe
to
88b7f60
Compare
mavam
changed the title
mavam
commented
Nov 19, 2019
mavam
force-pushed
the
story/ch4493
branch
2 times, most recently
from
05fdebe
to
887eecb
Compare
This index is meant for opaque identifiers that are can only be retrieved via equality lookups. If a string type contains the #id attribute in a schema, the index factory chooses a new hash-based implementation. The hash index is a vector of digests, where each digest is trimmed to a specific byte length. This works only if the hash function distributes its value uniformly over the entire output bits. Whether our current choice of hash function (xxhash64) is an open question that needs to figured out.
The value index keeps two extra bitmaps independent of its implementation: a null bitmap and a mask bitmap. The null bitmap is for all nil values and the mask value used to be for all IDs that exist in the index *and* the nil values. This change makes the two bitmaps disjoint. The result is not only less bits overall, but also less bit-wise operations when performing a lookup, because the null bitmap no longer needs to be NANDed out.
mavam
force-pushed
the
story/ch4493
branch
2 times, most recently
from
a0dcc1c
to
3ca898e
Compare
tobim
reviewed
Jan 9, 2020
dominiklohmann
requested changes
Jan 9, 2020
tobim
requested changes
Jan 9, 2020
tobim
reviewed
Jan 9, 2020
|
The commit ede727e seems to have triggered an ADL correctness issue, causing the wrong operator to be called and thus the wrong type name to be displayed. That should likely be fixed in a separate PR. |
This only popped up because of an overload in the Zeek type printer that didn't get triggered because it lacked a const qualifier.
tobim
previously approved these changes
Jan 9, 2020
|
@dominiklohmann Sorry, I fixed the issue already before seeing your request to do this separately. |
dominiklohmann
approved these changes
Jan 10, 2020
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.
The new index is meant for opaque identifiers that can only be retrieved via equality lookups. If a string type contains the
#index=hashattribute, the index factory chooses this new hash-based implementation.