Skip to content

Commit

Permalink
feat(tree_index): more aggressive root node cleanup
Browse files Browse the repository at this point in the history 
`TreeIndex` now tries to flatten the tree if it sees a chance.
  • Loading branch information
@wvwwvwwv
wvwwvwwv committed Jan 29, 2024
1 parent be81822 commit ebe2e31
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 62 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Version 2

2.0.14

* More aggressive `TreeIndex` root node cleanup.

2.0.13

* Add `iter` to `Queue` and `Stack`.
Expand Down
75 changes: 42 additions & 33 deletions src/tree_index.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,34 +338,38 @@ where
K: Borrow<Q>,
Q: Ord + ?Sized,
{
let mut has_been_removed = false;
let mut removed = false;
loop {
let guard = Guard::new();
if let Some(root_ref) = self.root.load(Acquire, &guard).as_ref() {
match root_ref.remove_if::<_, _, _>(key, &mut condition, &mut (), &guard) {
Ok(r) => match r {
if let Ok(result) =
root_ref.remove_if::<_, _, _>(key, &mut condition, &mut (), &guard)
{
if matches!(result, RemoveResult::Cleanup) {
root_ref.cleanup_link(key, false, &guard);
}
match result {
RemoveResult::Success => return true,
RemoveResult::Cleanup => {
root_ref.cleanup_link(key, false, &guard);
return true;
}
RemoveResult::Retired => {
RemoveResult::Cleanup | RemoveResult::Retired => {
if Node::cleanup_root(&self.root, &mut (), &guard) {
return true;
}
has_been_removed = true;
removed = true;
}
RemoveResult::Fail => return has_been_removed,
RemoveResult::Frozen => (),
},
Err(removed) => {
if removed {
has_been_removed = true;
RemoveResult::Fail => {
if removed {
if Node::cleanup_root(&self.root, &mut (), &guard) {
return true;
}
} else {
return false;
}
}
RemoveResult::Frozen => (),
}
}
} else {
return has_been_removed;
return removed;
}
}
}
Expand All @@ -392,42 +396,48 @@ where
K: Borrow<Q>,
Q: Ord + ?Sized,
{
let mut has_been_removed = false;
let mut removed = false;
loop {
let mut async_wait = AsyncWait::default();
let mut async_wait_pinned = Pin::new(&mut async_wait);
{
let guard = Guard::new();
if let Some(root_ref) = self.root.load(Acquire, &guard).as_ref() {
match root_ref.remove_if::<_, _, _>(
if let Ok(result) = root_ref.remove_if::<_, _, _>(
key,
&mut condition,
&mut async_wait_pinned,
&guard,
) {
Ok(r) => match r {
if matches!(result, RemoveResult::Cleanup) {
root_ref.cleanup_link(key, false, &guard);
}
match result {
RemoveResult::Success => return true,
RemoveResult::Cleanup => {
root_ref.cleanup_link(key, false, &guard);
return true;
}
RemoveResult::Retired => {
RemoveResult::Cleanup | RemoveResult::Retired => {
if Node::cleanup_root(&self.root, &mut async_wait_pinned, &guard) {
return true;
}
has_been_removed = true;
removed = true;
}
RemoveResult::Fail => return has_been_removed,
RemoveResult::Frozen => (),
},
Err(removed) => {
if removed {
has_been_removed = true;
RemoveResult::Fail => {
if removed {
if Node::cleanup_root(
&self.root,
&mut async_wait_pinned,
&guard,
) {
return true;
}
} else {
return false;
}
}
RemoveResult::Frozen => (),
}
}
} else {
return has_been_removed;
return removed;
}
}
async_wait_pinned.await;
Expand Down Expand Up @@ -482,7 +492,6 @@ where
for (k, _) in self.range(range, &Guard::new()) {
self.remove(k);
}
let _result = Node::cleanup_root(&self.root, &mut (), &Guard::new());
}

/// Returns a guarded reference to the value for the specified key without acquiring locks.
Expand Down
18 changes: 7 additions & 11 deletions src/tree_index/internal_node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ where
condition: &mut F,
async_wait: &mut D,
guard: &Guard,
) -> Result<RemoveResult, bool>
) -> Result<RemoveResult, ()>
where
K: Borrow<Q>,
Q: Ord + ?Sized,
Expand Down Expand Up @@ -1296,23 +1296,19 @@ mod test {
if max_key.map_or(false, |m| m == id) {
break;
}
let mut removed = false;
loop {
match internal_node_clone.remove_if::<_, _, _>(
if let Ok(r) = internal_node_clone.remove_if::<_, _, _>(
&id,
&mut |_| true,
&mut (),
&guard,
) {
Ok(r) => match r {
RemoveResult::Success | RemoveResult::Cleanup => break,
RemoveResult::Fail => {
assert!(removed);
break;
}
match r {
RemoveResult::Success
| RemoveResult::Cleanup
| RemoveResult::Fail => break,
RemoveResult::Frozen | RemoveResult::Retired => unreachable!(),
},
Err(r) => removed |= r,
}
}
}
assert!(internal_node_clone.search(&id, &guard).is_none());
Expand Down
22 changes: 22 additions & 0 deletions src/tree_index/leaf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,23 @@ where
Dimension::retired(self.metadata.load(Relaxed))
}

/// Returns `true` if the [`Leaf`] has no reachable entry.
#[inline]
pub(super) fn is_empty(&self) -> bool {
let mut mutable_metadata = self.metadata.load(Relaxed);
for _ in 0..DIMENSION.num_entries {
if mutable_metadata == 0 {
break;
}
let rank = mutable_metadata % (1_usize << DIMENSION.num_bits_per_entry);
if rank != Dimension::uninit_rank() && rank != DIMENSION.removed_rank() {
return false;
}
mutable_metadata >>= DIMENSION.num_bits_per_entry;
}
true
}

/// Returns a reference to the max key.
#[inline]
pub(super) fn max_key(&self) -> Option<&K> {
Expand Down Expand Up @@ -1098,6 +1115,7 @@ mod test {
#[test]
fn prop(insert in 0_usize..DIMENSION.num_entries, remove in 0_usize..DIMENSION.num_entries) {
let leaf: Leaf<usize, usize> = Leaf::new();
assert!(leaf.is_empty());
for i in 0..insert {
assert!(matches!(leaf.insert(i, i), InsertResult::Success));
if i != 0 {
Expand All @@ -1108,11 +1126,14 @@ mod test {
}
if insert == 0 {
assert_eq!(leaf.max_key(), None);
assert!(leaf.is_empty());
} else {
assert_eq!(leaf.max_key(), Some(&(insert - 1)));
assert!(!leaf.is_empty());
}
for i in 0..insert {
assert!(matches!(leaf.insert(i, i), InsertResult::Duplicate(..)));
assert!(!leaf.is_empty());
let result = leaf.min_greater_equal(&i);
assert_eq!(result.0, Some((&i, &i)));
}
Expand All @@ -1134,6 +1155,7 @@ mod test {
}
} else {
assert!(matches!(leaf.remove_if(&i, &mut |_| true), RemoveResult::Fail));
assert!(leaf.is_empty());
}
}
}
Expand Down
24 changes: 10 additions & 14 deletions src/tree_index/leaf_node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ where
condition: &mut F,
async_wait: &mut D,
guard: &Guard,
) -> Result<RemoveResult, bool>
) -> Result<RemoveResult, ()>
where
K: Borrow<Q>,
Q: Ord + ?Sized,
Expand All @@ -370,7 +370,7 @@ where
// When a `Leaf` is frozen, its entries may be being copied to new
// `Leaves`.
self.wait(async_wait);
return Err(false);
return Err(());
} else if result == RemoveResult::Retired {
return Ok(self.coalesce(guard));
}
Expand All @@ -390,7 +390,7 @@ where
let result = unbounded.remove_if(key, condition);
if result == RemoveResult::Frozen {
self.wait(async_wait);
return Err(false);
return Err(());
} else if result == RemoveResult::Retired {
return Ok(self.coalesce(guard));
}
Expand Down Expand Up @@ -635,7 +635,7 @@ where

/// Cleans up logically deleted [`LeafNode`] instances in the linked list.
///
/// If the target leaf node does not exist in the sub-tree, returns `false`.
/// If the target leaf does not exist in the [`LeafNode`], returns `false`.
#[inline]
pub(super) fn cleanup_link<'g, Q>(&self, key: &Q, tranverse_max: bool, guard: &'g Guard) -> bool
where
Expand Down Expand Up @@ -1197,23 +1197,19 @@ mod test {
if max_key.map_or(false, |m| m == id) {
break;
}
let mut removed = false;
loop {
match leaf_node_clone.remove_if::<_, _, _>(
if let Ok(r) = leaf_node_clone.remove_if::<_, _, _>(
&id,
&mut |_| true,
&mut (),
&guard,
) {
Ok(r) => match r {
RemoveResult::Success | RemoveResult::Cleanup => break,
RemoveResult::Fail => {
assert!(removed);
break;
}
match r {
RemoveResult::Success
| RemoveResult::Cleanup
| RemoveResult::Fail => break,
RemoveResult::Frozen | RemoveResult::Retired => unreachable!(),
},
Err(r) => removed |= r,
}
}
}
assert!(leaf_node_clone.search(&id, &guard).is_none(), "{}", id);
Expand Down
14 changes: 10 additions & 4 deletions src/tree_index/node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ where
condition: &mut F,
async_wait: &mut D,
guard: &Guard,
) -> Result<RemoveResult, bool>
) -> Result<RemoveResult, ()>
where
K: Borrow<Q>,
Q: Ord + ?Sized,
Expand Down Expand Up @@ -233,14 +233,20 @@ where
let mut leaf_node_locker = None;
match root_ref {
Self::Internal(internal_node) => {
if let Some(locker) = internal_node::Locker::try_lock(internal_node) {
if !internal_node.retired(Relaxed) && !internal_node.children.is_empty() {
// The internal node is still usable.
break;
} else if let Some(locker) = internal_node::Locker::try_lock(internal_node) {
internal_node_locker.replace(locker);
} else {
internal_node.wait(async_wait);
}
}
Self::Leaf(leaf_node) => {
if let Some(locker) = leaf_node::Locker::try_lock(leaf_node) {
if !leaf_node.retired(Relaxed) {
// The leaf node is still usable.
break;
} else if let Some(locker) = leaf_node::Locker::try_lock(leaf_node) {
leaf_node_locker.replace(locker);
} else {
leaf_node.wait(async_wait);
Expand All @@ -265,7 +271,7 @@ where
if internal_node.retired(Relaxed) {
// The internal node is empty, therefore the entire tree can be emptied.
None
} else if internal_node.children.max_key().is_none() {
} else if internal_node.children.is_empty() {
// Replace the root with the unbounded child.
internal_node.unbounded_child.get_shared(Acquire, guard)
} else {
Expand Down

0 comments on commit ebe2e31

Please sign in to comment.