Skip to content

Commit

Permalink
Merge #46
Browse files Browse the repository at this point in the history
46: Add ability to gc builder cache r=CAD97 a=CAD97

Draft while blocked on rust-lang/hashbrown#163

Also likely useful along these lines: a preload function that takes an `ArcBorrow<'_, Node>` and preloads the cache with the node and any transitive children.

`drain_filter` is annoying -- what's the polarity of the filter? -- so this definitely needs a test to make sure it's keeping the correct elements.

Co-authored-by: CAD97 <[email protected]>
  • Loading branch information
bors[bot] and CAD97 authored Jun 18, 2020
2 parents 2b03990 + afb98b8 commit aa056fd
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 3 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ license = "Apache-2.0 / MIT"
[dependencies]
ahash = "0.3"
erasable = "1.2" # public
hashbrown = "0.7"
hashbrown = "0.8"
ptr-union = "2.1"
rc-borrow = "1.3" # public
rc-box = { version = "1.1", features = ["slice-dst"] }
Expand Down
44 changes: 44 additions & 0 deletions src/green/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ impl Builder {
Self::default()
}

/// The number of cached elements.
pub fn size(&self) -> usize {
self.nodes.len() + self.tokens.len()
}
}

impl Builder {
/// Create a new node or clone a new Arc to an existing equivalent one.
///
/// This checks children for identity equivalence, not structural,
Expand Down Expand Up @@ -151,6 +158,43 @@ impl Builder {
}
}

impl Builder {
fn turn_node_gc(&mut self) -> bool {
// NB: `drain_filter` is `retain` but with an iterator of the removed elements.
// i.e.: elements where the predicate is FALSE are removed and iterated over.
self.nodes.drain_filter(|ThinEqNode(node), ()| Arc::strong_count(node) > 1).any(|_| true)
}

fn turn_token_gc(&mut self) -> bool {
self.tokens.drain_filter(|token, ()| Arc::strong_count(token) > 1).any(|_| true)
}

/// Collect cached nodes that are no longer live outside the cache.
///
/// This is a single turn of the GC, and may not GC all potentially unused
/// nodes in the cache. To run this to a fixpoint, use [`Builder::gc`].
pub fn turn_gc(&mut self) -> bool {
let removed_nodes = self.turn_node_gc();
let removed_tokens = self.turn_token_gc();
removed_nodes || removed_tokens
}

/// Collect all cached nodes that are no longer live outside the cache.
///
/// This is slightly more efficient than just running [`Builder::turn_gc`]
/// to a fixpoint, as it knows more about the cache structure and can avoid
/// re-GCing definitely clean sections.
pub fn gc(&mut self) {
// Nodes can keep other elements live, so GC them to a fixpoint
while self.turn_node_gc() {
continue;
}
// Tokens are guaranteed leaves, so only need a single GC turn
self.turn_token_gc();
debug_assert!(!self.turn_token_gc());
}
}

/// Checkpoint for maybe wrapping a node. See [`TreeBuilder::checkpoint`].
#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
pub struct Checkpoint(usize);
Expand Down
33 changes: 33 additions & 0 deletions tests/gc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use sorbus::*;

#[test]
fn works_properly() {
let kind0 = Kind(0);
let kind1 = Kind(1);
let kind2 = Kind(2);
let mut builder = green::TreeBuilder::new();

#[rustfmt::skip]
let inner = builder
.start_node(kind1)
.token(kind0, "kind")
.finish_node()
.finish();

#[rustfmt::skip]
let outer = builder
.start_node(kind2)
.add(inner.clone())
.finish_node()
.finish();

assert_eq!(builder.builder().size(), 3);

drop(outer);
builder.builder().gc();
assert_eq!(builder.builder().size(), 2);

drop(inner);
builder.builder().gc();
assert_eq!(builder.builder().size(), 0);
}

0 comments on commit aa056fd

Please sign in to comment.