Remove allocation for builder cache hits

src/green/builder.rs

@@ -18,24 +18,28 @@ struct ThinEqNode(Arc<Node>);
 impl Eq for ThinEqNode {}
 impl PartialEq for ThinEqNode {
     fn eq(&self, other: &Self) -> bool {
+        // we can skip `len` as it is derived from `children`
         self.0.kind() == other.0.kind()
-            // we can skip `len` as it is derived from `children`
             && self.0.children().zip(other.0.children()).all(|pair| match pair {
-                (NodeOrToken::Node(lhs), NodeOrToken::Node(rhs)) => ptr::eq(&*lhs, &*rhs),
-                (NodeOrToken::Token(lhs), NodeOrToken::Token(rhs)) => ptr::eq(&*lhs, &*rhs),
+                (NodeOrToken::Node(lhs), NodeOrToken::Node(rhs)) => {
+                    ptr::eq(&*lhs as *const Node, &*rhs as *const Node)
+                }
+                (NodeOrToken::Token(lhs), NodeOrToken::Token(rhs)) => {
+                    ptr::eq(&*lhs as *const Token, &*rhs as *const Token)
+                }
                 _ => false,
             })
     }
 }
 
 impl Hash for ThinEqNode {
     fn hash<H: Hasher>(&self, state: &mut H) {
-        self.0.kind().hash(state);
         // we can skip `len` as it is derived from `children`
+        self.0.kind().hash(state);
         for child in self.0.children() {
             match child {
-                NodeOrToken::Node(node) => ptr::hash(&*node, state),
-                NodeOrToken::Token(token) => ptr::hash(&*token, state),
+                NodeOrToken::Node(node) => ptr::hash(&*node as *const Node as *const (), state),
+                NodeOrToken::Token(token) => ptr::hash(&*token as *const Token as *const (), state),
             }
         }
     }
@@ -71,7 +75,7 @@ impl fmt::Debug for Builder {
     }
 }
 
-fn do_hash(hasher: &impl BuildHasher, hashee: &impl Hash) -> u64 {
+fn do_hash(hasher: &impl BuildHasher, hashee: &(impl ?Sized + Hash)) -> u64 {
     let state = &mut hasher.build_hasher();
     hashee.hash(state);
     state.finish()
@@ -90,20 +94,83 @@ impl Builder {
     /// if the lower-level nodes have also been cached.
     pub fn node<I>(&mut self, kind: Kind, children: I) -> Arc<Node>
     where
-        I: IntoIterator,
-        I::Item: Into<NodeOrToken<Arc<Node>, Arc<Token>>>,
-        I::IntoIter: ExactSizeIterator,
+        I: IntoIterator<Item = NodeOrToken<Arc<Node>, Arc<Token>>>,
+        I::IntoIter: ExactSizeIterator + AsRef<[NodeOrToken<Arc<Node>, Arc<Token>>]>,
     {
-        self.node_packed(kind, children.into_iter().map(Into::into).map(pack_node_or_token))
+        let hasher = &self.hasher;
+        let children = children.into_iter();
+
+        let hash = {
+            // spoof ThinNodeEq's hash impl
+            let state = &mut hasher.build_hasher();
+            kind.hash(state);
+            for child in children.as_ref() {
+                match child {
+                    NodeOrToken::Node(node) => {
+                        ptr::hash(&**node as *const Node as *const (), state)
+                    }
+                    NodeOrToken::Token(token) => {
+                        ptr::hash(&**token as *const Token as *const (), state)
+                    }
+                }
+            }
+            state.finish()
+        };
+
+        let entry = self.nodes.raw_entry_mut().from_hash(hash, |ThinEqNode(node)| {
+            node.kind() == kind
+                && node.children().zip(children.as_ref().iter()).all(|pair| match pair {
+                    (NodeOrToken::Node(lhs), NodeOrToken::Node(rhs)) => ptr::eq(&*lhs, &**rhs),
+                    (NodeOrToken::Token(lhs), NodeOrToken::Token(rhs)) => ptr::eq(&*lhs, &**rhs),
+                    _ => false,
+                })
+        });
+        let (ThinEqNode(node), ()) = match entry {
+            RawEntryMut::Occupied(entry) => entry.into_key_value(),
+            RawEntryMut::Vacant(entry) => {
+                let node = Node::new(kind, children.map(pack_node_or_token));
+                entry.insert_with_hasher(hash, ThinEqNode(node), (), |x| do_hash(hasher, x))
+            }
+        };
+        Arc::clone(node)
     }
 
     /// Version of `Builder::node` taking a pre-packed child element iterator.
     pub(super) fn node_packed<I>(&mut self, kind: Kind, children: I) -> Arc<Node>
     where
-        I: Iterator<Item = PackedNodeOrToken> + ExactSizeIterator,
+        I: Iterator<Item = PackedNodeOrToken> + ExactSizeIterator + AsRef<[PackedNodeOrToken]>,
     {
-        let node = Node::new(kind, children);
-        self.cache_node(node)
+        let hasher = &self.hasher;
+
+        let hash = {
+            // spoof ThinNodeEq's hash impl
+            let state = &mut hasher.build_hasher();
+            kind.hash(state);
+            for child in children.as_ref() {
+                ptr::hash(child.as_untagged_ptr().as_ptr(), state);
+            }
+            state.finish()
+        };
+
+        let entry = self.nodes.raw_entry_mut().from_hash(hash, |ThinEqNode(node)| {
+            node.kind() == kind
+                && node.children().zip(children.as_ref().iter()).all(|(lhs, rhs)| match lhs {
+                    NodeOrToken::Node(lhs) => {
+                        ptr::eq((&*lhs) as *const _ as *const _, rhs.as_untagged_ptr().as_ptr())
+                    }
+                    NodeOrToken::Token(lhs) => {
+                        ptr::eq((&*lhs) as *const _ as *const _, rhs.as_untagged_ptr().as_ptr())
+                    }
+                })
+        });
+        let (ThinEqNode(node), ()) = match entry {
+            RawEntryMut::Occupied(entry) => entry.into_key_value(),
+            RawEntryMut::Vacant(entry) => {
+                let node = Node::new(kind, children);
+                entry.insert_with_hasher(hash, ThinEqNode(node), (), |x| do_hash(hasher, x))
+            }
+        };
+        Arc::clone(node)
     }
 
     /// Get a cached version of the input node.
@@ -194,9 +261,8 @@ impl TreeBuilder {
     /// Add a new node to the current branch.
     pub fn node<I>(&mut self, kind: Kind, children: I) -> &mut Self
     where
-        I: IntoIterator,
-        I::Item: Into<NodeOrToken<Arc<Node>, Arc<Token>>>,
-        I::IntoIter: ExactSizeIterator,
+        I: IntoIterator<Item = NodeOrToken<Arc<Node>, Arc<Token>>>,
+        I::IntoIter: ExactSizeIterator + AsRef<[NodeOrToken<Arc<Node>, Arc<Token>>]>,
     {
         let node = self.cache.node(kind, children);
         self.add(node)

src/lib.rs

@@ -9,6 +9,7 @@
 
 #![forbid(unconditional_recursion)]
 #![warn(missing_debug_implementations, missing_docs)]
+#![allow(clippy::unnested_or_patterns)] // rust-lang/rust-clippy#5702
 
 #[cfg(not(any(target_pointer_width = "32", target_pointer_width = "64")))]
 compile_error!("sorbus only works when sizeof(*const ()) is u32 or u64");

tests/smoke.rs

@@ -108,16 +108,15 @@ fn make_math_tree() {
     // Invocations of the builder with the same (id) arguments produces the same (id) results
     assert!(Arc::ptr_eq(&ws, &builder.token(WS, " ")));
 
-    // builder.node accepts iterator of Arc<Node>, Arc<Token>, or NodeOrToken<Arc<Node>, Arc<Token>>
-    // so if you're mixing nodes and tokens, you need to include the type changing boilerplate.
+    // builder.node accepts iterator of NodeOrToken<Arc<Node>, Arc<Token>> so you need to wrap.
     // You'll know if you need the bottom-up builder (LR or such). Use TreeBuilder otherwise.
     let n = |node: &Arc<green::Node>| NodeOrToken::from(node.clone());
     let t = |token: &Arc<green::Token>| NodeOrToken::from(token.clone());
 
-    // We use vec![] as a quick and easy ExactSizeIterator.
-    // Particular implementations may use specialized iterators for known child array lengths.
-    // (Please, const-generic angels, give us `[_; N]: IntoIterator` sooner rather than later!)
-    let inner_mul = builder.node(EXPR, vec![n2, ws.clone(), mul, ws.clone(), n3]);
+    // Currently, only vec::IntoIter and vec::Drain are able to be used for builder.node, as
+    // building a node requires being able to pre-iterate the children to check the dedup cache.
+    // (Please, const-generic angels, give us `for<const N: usize> [_; N]: IntoIterator` soon(ish)!)
+    let inner_mul = builder.node(EXPR, vec![t(&n2), t(&ws), t(&mul), t(&ws), t(&n3)]);
     let left_add = builder.node(EXPR, vec![t(&n1), t(&ws), t(&add), t(&ws), n(&inner_mul)]);
     let right_add = builder.node(EXPR, vec![n(&left_add), t(&ws), t(&add), t(&ws), t(&n4)]);