Skip to content

Commit 47bed0c

Browse files
apoelstraapoelstra
committed
iter: get rid of Arc::clones and allocations for n-ary nodes
This somewhat complicates implementation of the trait, but not by much, and eliminates a ton of refcount accesses and increments, and a ton of Arc::clones. As we will see, having an Arc<[T]> is somewhat restrictive in addition to requiring allocations. Later we will want to replace this with an enum that covers multiple kinds of arrays. I was unsure whether to name the new associated type NAry or Nary. It seems more natural to use NAry but the existing Tree::Nary variant uses the lowercase spelling so I stuck with that. (And similarly used nary_ for the method names rather than n_ary.)
1 parent 67d6ff7 commit 47bed0c

File tree

4 files changed

+120
-42
lines changed

4 files changed

+120
-42
lines changed

‎src/iter/mod.rs

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@ use crate::sync::Arc;
1818
use crate::{Miniscript, MiniscriptKey, ScriptContext, Terminal};
1919

2020
impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Miniscript<Pk, Ctx> {
21-
fn as_node(&self) -> Tree<Self> {
21+
type NaryChildren = &'a [Arc<Miniscript<Pk, Ctx>>];
22+
23+
fn nary_len(tc: &Self::NaryChildren) -> usize { tc.len() }
24+
fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { Arc::as_ref(&tc[idx]) }
25+
26+
fn as_node(&self) -> Tree<Self, Self::NaryChildren> {
2227
use Terminal::*;
2328
match self.node {
2429
PkK(..) | PkH(..) | RawPkH(..) | After(..) | Older(..) | Sha256(..) | Hash256(..)
@@ -37,13 +42,18 @@ impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Miniscript<Pk,
3742
| OrC(ref left, ref right)
3843
| OrI(ref left, ref right) => Tree::Binary(left, right),
3944
AndOr(ref a, ref b, ref c) => Tree::Ternary(a, b, c),
40-
Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::as_ref).collect()),
45+
Thresh(ref thresh) => Tree::Nary(thresh.data()),
4146
}
4247
}
4348
}
4449

45-
impl<Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for Arc<Miniscript<Pk, Ctx>> {
46-
fn as_node(&self) -> Tree<Self> {
50+
impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Arc<Miniscript<Pk, Ctx>> {
51+
type NaryChildren = &'a [Arc<Miniscript<Pk, Ctx>>];
52+
53+
fn nary_len(tc: &Self::NaryChildren) -> usize { tc.len() }
54+
fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { &tc[idx] }
55+
56+
fn as_node(&self) -> Tree<Self, Self::NaryChildren> {
4757
use Terminal::*;
4858
match self.node {
4959
PkK(..) | PkH(..) | RawPkH(..) | After(..) | Older(..) | Sha256(..) | Hash256(..)
@@ -54,17 +64,15 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for Arc<Miniscript<Pk, Ctx>
5464
| DupIf(ref sub)
5565
| Verify(ref sub)
5666
| NonZero(ref sub)
57-
| ZeroNotEqual(ref sub) => Tree::Unary(Arc::clone(sub)),
67+
| ZeroNotEqual(ref sub) => Tree::Unary(sub),
5868
AndV(ref left, ref right)
5969
| AndB(ref left, ref right)
6070
| OrB(ref left, ref right)
6171
| OrD(ref left, ref right)
6272
| OrC(ref left, ref right)
63-
| OrI(ref left, ref right) => Tree::Binary(Arc::clone(left), Arc::clone(right)),
64-
AndOr(ref a, ref b, ref c) => {
65-
Tree::Ternary(Arc::clone(a), Arc::clone(b), Arc::clone(c))
66-
}
67-
Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::clone).collect()),
73+
| OrI(ref left, ref right) => Tree::Binary(left, right),
74+
AndOr(ref a, ref b, ref c) => Tree::Ternary(a, b, c),
75+
Thresh(ref thresh) => Tree::Nary(thresh.data()),
6876
}
6977
}
7078
}

‎src/iter/tree.rs

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,12 @@
77
//!
88
99
use crate::prelude::*;
10-
use crate::sync::Arc;
1110

1211
/// Abstract node of a tree.
1312
///
1413
/// Tracks the arity (out-degree) of a node, which is the only thing that
1514
/// is needed for iteration purposes.
16-
pub enum Tree<T> {
15+
pub enum Tree<T, NT> {
1716
/// Combinator with no children.
1817
Nullary,
1918
/// Combinator with one child.
@@ -23,7 +22,7 @@ pub enum Tree<T> {
2322
/// Combinator with two children.
2423
Ternary(T, T, T),
2524
/// Combinator with more than two children.
26-
Nary(Arc<[T]>),
25+
Nary(NT),
2726
}
2827

2928
/// A trait for any structure which has the shape of a Miniscript tree.
@@ -32,14 +31,27 @@ pub enum Tree<T> {
3231
/// rather than nodes themselves, because it provides algorithms that
3332
/// assume copying is cheap.
3433
///
35-
/// To implement this trait, you only need to implement the [`TreeLike::as_node`]
36-
/// method, which will usually be very mechanical. Everything else is provided.
37-
/// However, to avoid allocations, it may make sense to also implement
38-
/// [`TreeLike::n_children`] and [`TreeLike::nth_child`] because the default
39-
/// implementations will allocate vectors for n-ary nodes.
34+
/// To implement this trait, you only need to implement the [`TreeLike::as_node`],
35+
/// [`TreeLike::nary_len`] and `[TreeLike::nary_index'] methods, which should
36+
/// be very mechanical. Everything else is provided.
4037
pub trait TreeLike: Clone + Sized {
38+
/// An abstraction over the children of n-ary nodes. Typically when
39+
/// implementing the trait for `&a T` this will be `&'a [T]`.
40+
type NaryChildren: Clone;
41+
42+
/// Accessor for the length of a [`Self::NaryChildren`].
43+
fn nary_len(tc: &Self::NaryChildren) -> usize;
44+
45+
/// Accessor for a specific child of a [`Self::NaryChildren`].
46+
///
47+
/// # Panics
48+
///
49+
/// May panic if asked for an element outside of the range
50+
/// `0..Self::nary_len(&tc)`.
51+
fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self;
52+
4153
/// Interpret the node as an abstract node.
42-
fn as_node(&self) -> Tree<Self>;
54+
fn as_node(&self) -> Tree<Self, Self::NaryChildren>;
4355

4456
/// Accessor for the number of children this node has.
4557
fn n_children(&self) -> usize {
@@ -48,7 +60,7 @@ pub trait TreeLike: Clone + Sized {
4860
Tree::Unary(..) => 1,
4961
Tree::Binary(..) => 2,
5062
Tree::Ternary(..) => 3,
51-
Tree::Nary(children) => children.len(),
63+
Tree::Nary(ref children) => Self::nary_len(children),
5264
}
5365
}
5466

@@ -65,7 +77,10 @@ pub trait TreeLike: Clone + Sized {
6577
(1, Tree::Ternary(_, sub, _)) => Some(sub),
6678
(2, Tree::Ternary(_, _, sub)) => Some(sub),
6779
(_, Tree::Ternary(..)) => None,
68-
(n, Tree::Nary(children)) => children.get(n).cloned(),
80+
(n, Tree::Nary(children)) if n < Self::nary_len(&children) => {
81+
Some(Self::nary_index(children, n))
82+
}
83+
(_, Tree::Nary(..)) => None,
6984
}
7085
}
7186

@@ -223,7 +238,9 @@ impl<T: TreeLike> Iterator for PreOrderIter<T> {
223238
self.stack.push(a);
224239
}
225240
Tree::Nary(children) => {
226-
self.stack.extend(children.iter().rev().cloned());
241+
for i in (0..T::nary_len(&children)).rev() {
242+
self.stack.push(T::nary_index(children.clone(), i));
243+
}
227244
}
228245
}
229246
Some(top)

‎src/policy/concrete.rs

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
594594
};
595595
match new_policy {
596596
Some(new_policy) => translated.push(Arc::new(new_policy)),
597-
None => translated.push(Arc::clone(&data.node)),
597+
None => translated.push(Arc::clone(data.node)),
598598
}
599599
}
600600
// Ok to unwrap because we know we processed at least one node.
@@ -1044,30 +1044,73 @@ fn generate_combination<Pk: MiniscriptKey>(
10441044
ret
10451045
}
10461046

1047+
/// A type used within the iterator API to abstract over the two kinds
1048+
/// of n-ary nodes in a [`Policy`].
1049+
///
1050+
/// Generally speaking, users should never need to see or be aware of this
1051+
/// type. But when directly implementing iterators it may come in handy.
1052+
#[derive(Clone)]
1053+
pub enum TreeChildren<'a, Pk: MiniscriptKey> {
1054+
/// A conjunction or threshold node's children.
1055+
And(&'a [Arc<Policy<Pk>>]),
1056+
/// A disjunction node's children.
1057+
Or(&'a [(usize, Arc<Policy<Pk>>)]),
1058+
}
1059+
10471060
impl<'a, Pk: MiniscriptKey> TreeLike for &'a Policy<Pk> {
1048-
fn as_node(&self) -> Tree<Self> {
1061+
type NaryChildren = TreeChildren<'a, Pk>;
1062+
1063+
fn nary_len(tc: &Self::NaryChildren) -> usize {
1064+
match tc {
1065+
TreeChildren::And(sl) => sl.len(),
1066+
TreeChildren::Or(sl) => sl.len(),
1067+
}
1068+
}
1069+
fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self {
1070+
match tc {
1071+
TreeChildren::And(sl) => &sl[idx],
1072+
TreeChildren::Or(sl) => &sl[idx].1,
1073+
}
1074+
}
1075+
1076+
fn as_node(&self) -> Tree<Self, Self::NaryChildren> {
10491077
use Policy::*;
10501078

10511079
match *self {
10521080
Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_)
10531081
| Ripemd160(_) | Hash160(_) => Tree::Nullary,
1054-
And(ref subs) => Tree::Nary(subs.iter().map(Arc::as_ref).collect()),
1055-
Or(ref v) => Tree::Nary(v.iter().map(|(_, p)| p.as_ref()).collect()),
1056-
Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::as_ref).collect()),
1082+
And(ref subs) => Tree::Nary(TreeChildren::And(subs)),
1083+
Or(ref v) => Tree::Nary(TreeChildren::Or(v)),
1084+
Thresh(ref thresh) => Tree::Nary(TreeChildren::And(thresh.data())),
10571085
}
10581086
}
10591087
}
10601088

1061-
impl<Pk: MiniscriptKey> TreeLike for Arc<Policy<Pk>> {
1062-
fn as_node(&self) -> Tree<Self> {
1089+
impl<'a, Pk: MiniscriptKey> TreeLike for &'a Arc<Policy<Pk>> {
1090+
type NaryChildren = TreeChildren<'a, Pk>;
1091+
1092+
fn nary_len(tc: &Self::NaryChildren) -> usize {
1093+
match tc {
1094+
TreeChildren::And(sl) => sl.len(),
1095+
TreeChildren::Or(sl) => sl.len(),
1096+
}
1097+
}
1098+
fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self {
1099+
match tc {
1100+
TreeChildren::And(sl) => &sl[idx],
1101+
TreeChildren::Or(sl) => &sl[idx].1,
1102+
}
1103+
}
1104+
1105+
fn as_node(&self) -> Tree<Self, Self::NaryChildren> {
10631106
use Policy::*;
10641107

1065-
match self.as_ref() {
1108+
match ***self {
10661109
Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_)
10671110
| Ripemd160(_) | Hash160(_) => Tree::Nullary,
1068-
And(ref subs) => Tree::Nary(subs.iter().map(Arc::clone).collect()),
1069-
Or(ref v) => Tree::Nary(v.iter().map(|(_, p)| Arc::clone(p)).collect()),
1070-
Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::clone).collect()),
1111+
And(ref subs) => Tree::Nary(TreeChildren::And(subs)),
1112+
Or(ref v) => Tree::Nary(TreeChildren::Or(v)),
1113+
Thresh(ref thresh) => Tree::Nary(TreeChildren::And(thresh.data())),
10711114
}
10721115
}
10731116
}

‎src/policy/semantic.rs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
499499
};
500500
match new_policy {
501501
Some(new_policy) => at_age.push(Arc::new(new_policy)),
502-
None => at_age.push(Arc::clone(&data.node)),
502+
None => at_age.push(Arc::clone(data.node)),
503503
}
504504
}
505505
// Unwrap is ok because we know we processed at least one node.
@@ -531,7 +531,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
531531
};
532532
match new_policy {
533533
Some(new_policy) => at_age.push(Arc::new(new_policy)),
534-
None => at_age.push(Arc::clone(&data.node)),
534+
None => at_age.push(Arc::clone(data.node)),
535535
}
536536
}
537537
// Unwrap is ok because we know we processed at least one node.
@@ -609,7 +609,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
609609
};
610610
match new_policy {
611611
Some(new_policy) => sorted.push(Arc::new(new_policy)),
612-
None => sorted.push(Arc::clone(&data.node)),
612+
None => sorted.push(Arc::clone(data.node)),
613613
}
614614
}
615615
// Unwrap is ok because we know we processed at least one node.
@@ -620,25 +620,35 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
620620
}
621621

622622
impl<'a, Pk: MiniscriptKey> TreeLike for &'a Policy<Pk> {
623-
fn as_node(&self) -> Tree<Self> {
623+
type NaryChildren = &'a [Arc<Policy<Pk>>];
624+
625+
fn nary_len(tc: &Self::NaryChildren) -> usize { tc.len() }
626+
fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { &tc[idx] }
627+
628+
fn as_node(&self) -> Tree<Self, Self::NaryChildren> {
624629
use Policy::*;
625630

626631
match *self {
627632
Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_)
628633
| Ripemd160(_) | Hash160(_) => Tree::Nullary,
629-
Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::as_ref).collect()),
634+
Thresh(ref thresh) => Tree::Nary(thresh.data()),
630635
}
631636
}
632637
}
633638

634-
impl<Pk: MiniscriptKey> TreeLike for Arc<Policy<Pk>> {
635-
fn as_node(&self) -> Tree<Self> {
639+
impl<'a, Pk: MiniscriptKey> TreeLike for &'a Arc<Policy<Pk>> {
640+
type NaryChildren = &'a [Arc<Policy<Pk>>];
641+
642+
fn nary_len(tc: &Self::NaryChildren) -> usize { tc.len() }
643+
fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { &tc[idx] }
644+
645+
fn as_node(&self) -> Tree<Self, Self::NaryChildren> {
636646
use Policy::*;
637647

638-
match self.as_ref() {
648+
match ***self {
639649
Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_)
640650
| Ripemd160(_) | Hash160(_) => Tree::Nullary,
641-
Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::clone).collect()),
651+
Thresh(ref thresh) => Tree::Nary(thresh.data()),
642652
}
643653
}
644654
}

0 commit comments

Comments
 (0)