Conversation
**Motivation** The main `representation` module is quite large, and grouping all the pointers together in a sub-module makes for a slightly easier-to-read source code. **Testing Done** `./scripts/full-test.sh nightly`
**Description** - Separate indirect node indexing into sub-module - Refactor all `grow`/`shrink` methods out into `From` implementations that are outside of the `InnerNode` trait - Rework the `RestrictedNodeIndex` type into `Option<NonMaxIndex>` - Switch `impl InnerNode for InnerNode48` to instead be generic over any `InnerNodeIndirect`. **Motivation** - The indexing code was self-contained, it made sense to separate into sub-module to restrict the API surface - Moving to the `From` implementations means that in the future it would be easier to add more inner node types and make them convert into each other. - Reworking the `RestrictedNodeIndex` simplifies the entire indexing scheme, as using `Option` is more common across Rust code. - Making the `impl InnerNode` generic over any size of `InnerNodeIndirect` makes it easier to add new inner node types in the future. **Testing Done** `./scripts/full-test.sh nightly`
**Description** Remove `SearchInnerNodeSorted` trait that was implemented for specific sizes of `InnerNodeSorted<..., SIZE>` and replace with a runtime check that dispatches to a function optimized for specific sizes. Pull out some common range bound validation code into a helper function. Switch from using `MaybeUninit<u8>` in the `InnerNodeSorted` type to plain `u8`, and update related code. Remove some unsound code that blanket converted from `[MaybeUninit<u8>; 16]` to `[u8; 16]` without confirming that all values were "init". **Motivation** The `SearchInnerNodeSorted` trait made it difficult to make the `impl InnerNode<...> for InnerNodeSorted<..., SIZE>` generic over `SIZE`, since it was implemented for specific values of `SIZE`. Helper function is good to reduce code duplication. The change from `MaybeUninit<u8>` to `u8` was to remove the complexity of dealing with those uninit values, specifically when using converting the keys array to SIMD types. I also think that we won't lose much performance, since the keys array is typically small and we don't gain much by leaving those values uninit. **Testing Done** `./scripts/full-test.sh nightly`
**Description** Split `InnerNode` trait into `InnerNode` and `InnerNodeCommon`. `InnerNodeCommon` has all the functions related to accessing and modifying the inner node. `InnerNode` is a sub-trait that adds on info about the growth order of the inner nodes and defines a specific `NodeType`. **Motivation** I made the split so that I could implement `InnerNodeCommon` for `InnerNodeSorted<..., SIZE>` and `InnerNodeIndirect<..., SIZE>` and have it be generic over `SIZE`. That wouldn't be possible if I had done that before the split, because the `NodeType` for each `InnerNode` impl needs to be specific to the `SIZE`. **Testing Done** `./scripts/full-test.sh nightly`
**Description** - Add the `match_concrete_node_pointer!` macro that makes code which dispatches over a `ConcreteNodePointer` value more compact - Reduce duplication in the `FuzzySearch` trait impls by making all `InnerNode`s share the same impl. **Motivation** - The dispatch macro centralizes some of the logic for dispatching fns over different kinds of `InnerNode`s and makes it easier to change those node types or add new ones in the future. - Code deduplication is nice **Testing** `./scripts/full-test.sh nightly`
**Description** - Modify the `Visitor` trait so that it is generic over the `InnerNode` kinds instead of having separate trait methods for each type. - Switch some existing unit tests to use the `expect_test` crate so that updating tests for complex outputs is simplified. **Motivation** - Making the `Visitor` trait independent of specific `InnerNode` types makes it easier to add new `InnerNode` types in the future. - Similarly, expect/snapshot tests are easy to update for future changes **Testing** `./scripts/full-test.sh nightly`
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
1 participant
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.
SearchInnerNodeSortedwith runtime dispatchInnerNodetraitVisitortrait generic over inner node types