Repository navigation
Conversation
When we need to split nodes in a bforest to perform an insertion, allocate the worst case depth + 1 nodes all up front so that the insertion atomically completes or fails. Otherwise the bforest can silently drop nodes that were in the tree.
adamrk
commented
Oct 9, 2026
Comment on lines
-1102
to
+1134
| assert_eq!(m.tpath(110, f, &()), "node2[0]--node0[0]"); | ||
| assert_eq!(m.tpath(140, f, &()), "node2[0]--node0[3]"); | ||
| assert_eq!(m.tpath(210, f, &()), "node2[1]--node1[0]"); | ||
| assert_eq!(m.tpath(270, f, &()), "node2[1]--node1[6]"); | ||
| assert_eq!(m.tpath(310, f, &()), "node2[2]--node3[0]"); | ||
| assert_eq!(m.tpath(810, f, &()), "node2[7]--node8[0]"); | ||
| assert_eq!(m.tpath(870, f, &()), "node2[7]--node8[6]"); | ||
| assert_eq!(m.tpath(110, f, &()), "node1[0]--node0[0]"); | ||
| assert_eq!(m.tpath(140, f, &()), "node1[0]--node0[3]"); | ||
| assert_eq!(m.tpath(210, f, &()), "node1[1]--node2[0]"); | ||
| assert_eq!(m.tpath(270, f, &()), "node1[1]--node2[6]"); | ||
| assert_eq!(m.tpath(310, f, &()), "node1[2]--node5[0]"); | ||
| assert_eq!(m.tpath(810, f, &()), "node1[7]--node10[0]"); | ||
| assert_eq!(m.tpath(870, f, &()), "node1[7]--node10[6]"); |
Member
Author
There was a problem hiding this comment.
A bunch of these tests change because the node ids are different now. This is because 1) when reserving, the nodes end up in the free list in the opposite order in which the were allocated and 2) we reserve the worst case each time instead of the exact amount needed. I was thinking that making those changes isn't worth the added complexity.
alexcrichton
requested review from
fitzgen
and removed request for
alexcrichton
October 9, 2026 20:34
Member
|
I'll defer this to @fitzgen as he's more familiar with this than I, but I'd recommend against a test-only bool to fail allocations and instead relying only on the OOM test/fuzz harness we have for handling that. Could the tests be moved over there for OOM-related things? |
This reverts commit 20206e5.
This branch has not been deployed
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
2 participants
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.
When we need to split nodes in a bforest to perform an insertion, allocate
the worst case depth + 1 nodes all up front so that the insertion
atomically completes or fails. Otherwise the bforest can drop nodes that
were in the tree.