Remove usage of explicit leaves #108
Merged
Conversation
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
Everything went as planned except that we needed to mock the child of
the deleted node when it is an implicit leaf.
The losses:
This change puts additional load on:
- delete_case{3,4,5},
- insert_case3, and
- rotate_{left,right}
This load comes from nullchecks before color checks and parent
assignments.
These checks should only be needed at the lowest levels of the fixed
path because of the red-black tree properties.
They could perhaps be circumvented entirely by adding additional mocked
leaves to the sibling of the child of the deleted node. This should
somehow be amortized by the facts that there is at most one deletion per
node created and that each node now costs less to create (because of the
allowed null pointers, see gains).
If property access is always preceded by a nullcheck and if an explicit
preceding nullcheck is somehow optimized away by the compiler, then perhaps
nothing needs to be done. We have to profile this.
The gains:
- all Leaf children are replaced by null (>1/6 space save)
- all isLeaf() calls are replaced by nullchecks: really faster? Should
be since I assume each method call must be preceded by a null check
internally?
Also we should rewrite delete_one_child to completely avoid this mocking
in case the deleted node is red. I left a TODO note about this.
This is progress on #104.
This avoids creating a mocked leaf in that case.
The coverage report seems to indicate that some additional pruning is possible in delete_one_child. Need to investigate.
Indeed the previous coverage report gave the right insight.
This avoids using multiple Hidden Classes and enables inline caching. Fixes #107.
This makes the config easier to parse.
The goal is to have case number zero for the base case of the recursion. I had to do each of these renaming operations individually as otherwise git would register a consequent amount of churn.
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.
Fixes #104, fixes #107.