From 426c34d4309e8c7f844958642e651ca68acdb0a8 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Tue, 5 Mar 2024 10:15:50 +0100 Subject: [PATCH 1/3] add conditional formatting for Terminal fmt functions produce big binaries. Since Debug and Display implementation of the match are equal except the Debug or Display representation of its child, this introduce conditional fmt trading performance for code size. The branch introduced should be easily predicted so the performance impact shouldn't be big. --- src/miniscript/astelem.rs | 275 +++++++++++++++++++------------------- 1 file changed, 134 insertions(+), 141 deletions(-) diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index e30765548..7a39b071d 100644 --- a/src/miniscript/astelem.rs +++ b/src/miniscript/astelem.rs @@ -41,160 +41,48 @@ impl Terminal { _ => None, } } -} - -impl fmt::Debug for Terminal { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str("[")?; - if let Ok(type_map) = types::Type::type_check(self) { - f.write_str(match type_map.corr.base { - types::Base::B => "B", - types::Base::K => "K", - types::Base::V => "V", - types::Base::W => "W", - })?; - fmt::Write::write_char(f, '/')?; - f.write_str(match type_map.corr.input { - types::Input::Zero => "z", - types::Input::One => "o", - types::Input::OneNonZero => "on", - types::Input::Any => "", - types::Input::AnyNonZero => "n", - })?; - if type_map.corr.dissatisfiable { - fmt::Write::write_char(f, 'd')?; - } - if type_map.corr.unit { - fmt::Write::write_char(f, 'u')?; - } - f.write_str(match type_map.mall.dissat { - types::Dissat::None => "f", - types::Dissat::Unique => "e", - types::Dissat::Unknown => "", - })?; - if type_map.mall.safe { - fmt::Write::write_char(f, 's')?; - } - if type_map.mall.non_malleable { - fmt::Write::write_char(f, 'm')?; - } - } else { - f.write_str("TYPECHECK FAILED")?; - } - f.write_str("]")?; - if let Some((ch, sub)) = self.wrap_char() { - fmt::Write::write_char(f, ch)?; - if sub.node.wrap_char().is_none() { - fmt::Write::write_char(f, ':')?; - } - write!(f, "{:?}", sub) - } else { - match *self { - Terminal::PkK(ref pk) => write!(f, "pk_k({:?})", pk), - Terminal::PkH(ref pk) => write!(f, "pk_h({:?})", pk), - Terminal::RawPkH(ref pkh) => write!(f, "expr_raw_pk_h({:?})", pkh), - Terminal::After(t) => write!(f, "after({})", t), - Terminal::Older(t) => write!(f, "older({})", t), - Terminal::Sha256(ref h) => write!(f, "sha256({})", h), - Terminal::Hash256(ref h) => write!(f, "hash256({})", h), - Terminal::Ripemd160(ref h) => write!(f, "ripemd160({})", h), - Terminal::Hash160(ref h) => write!(f, "hash160({})", h), - Terminal::True => f.write_str("1"), - Terminal::False => f.write_str("0"), - Terminal::AndV(ref l, ref r) => write!(f, "and_v({:?},{:?})", l, r), - Terminal::AndB(ref l, ref r) => write!(f, "and_b({:?},{:?})", l, r), - Terminal::AndOr(ref a, ref b, ref c) => { - if c.node == Terminal::False { - write!(f, "and_n({:?},{:?})", a, b) - } else { - write!(f, "andor({:?},{:?},{:?})", a, b, c) - } - } - Terminal::OrB(ref l, ref r) => write!(f, "or_b({:?},{:?})", l, r), - Terminal::OrD(ref l, ref r) => write!(f, "or_d({:?},{:?})", l, r), - Terminal::OrC(ref l, ref r) => write!(f, "or_c({:?},{:?})", l, r), - Terminal::OrI(ref l, ref r) => write!(f, "or_i({:?},{:?})", l, r), - Terminal::Thresh(k, ref subs) => { - write!(f, "thresh({}", k)?; - for s in subs { - write!(f, ",{:?}", s)?; - } - f.write_str(")") - } - Terminal::Multi(k, ref keys) => { - write!(f, "multi({}", k)?; - for k in keys { - write!(f, ",{:?}", k)?; - } - f.write_str(")") - } - Terminal::MultiA(k, ref keys) => { - write!(f, "multi_a({}", k)?; - for k in keys { - write!(f, ",{}", k)?; - } - f.write_str(")") - } - _ => unreachable!(), - } - } - } -} -impl fmt::Display for Terminal { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fn conditional_fmt(&self, f: &mut fmt::Formatter, is_debug: bool) -> fmt::Result { match *self { - Terminal::PkK(ref pk) => write!(f, "pk_k({})", pk), - Terminal::PkH(ref pk) => write!(f, "pk_h({})", pk), - Terminal::RawPkH(ref pkh) => write!(f, "expr_raw_pk_h({})", pkh), - Terminal::After(t) => write!(f, "after({})", t), - Terminal::Older(t) => write!(f, "older({})", t), - Terminal::Sha256(ref h) => write!(f, "sha256({})", h), - Terminal::Hash256(ref h) => write!(f, "hash256({})", h), - Terminal::Ripemd160(ref h) => write!(f, "ripemd160({})", h), - Terminal::Hash160(ref h) => write!(f, "hash160({})", h), + Terminal::PkK(ref pk) => fmt_1(f, "pk_k(", pk, is_debug), + Terminal::PkH(ref pk) => fmt_1(f, "pk_h(", pk, is_debug), + Terminal::RawPkH(ref pkh) => fmt_1(f, "expr_raw_pk_h(", pkh, is_debug), + Terminal::After(ref t) => fmt_1(f, "after(", t, is_debug), + Terminal::Older(ref t) => fmt_1(f, "older(", t, is_debug), + Terminal::Sha256(ref h) => fmt_1(f, "sha256(", h, is_debug), + Terminal::Hash256(ref h) => fmt_1(f, "hash256(", h, is_debug), + Terminal::Ripemd160(ref h) => fmt_1(f, "ripemd160(", h, is_debug), + Terminal::Hash160(ref h) => fmt_1(f, "hash160(", h, is_debug), Terminal::True => f.write_str("1"), Terminal::False => f.write_str("0"), Terminal::AndV(ref l, ref r) if r.node != Terminal::True => { - write!(f, "and_v({},{})", l, r) + fmt_2(f, "and_v(", l, r, is_debug) } - Terminal::AndB(ref l, ref r) => write!(f, "and_b({},{})", l, r), + Terminal::AndB(ref l, ref r) => fmt_2(f, "and_b(", l, r, is_debug), Terminal::AndOr(ref a, ref b, ref c) => { if c.node == Terminal::False { - write!(f, "and_n({},{})", a, b) + fmt_2(f, "and_b(", a, b, is_debug) } else { - write!(f, "andor({},{},{})", a, b, c) + f.write_str("andor(")?; + conditional_fmt(f, a, is_debug)?; + f.write_str(",")?; + conditional_fmt(f, b, is_debug)?; + f.write_str(",")?; + conditional_fmt(f, c, is_debug)?; + f.write_str(")") } } - Terminal::OrB(ref l, ref r) => write!(f, "or_b({},{})", l, r), - Terminal::OrD(ref l, ref r) => write!(f, "or_d({},{})", l, r), - Terminal::OrC(ref l, ref r) => write!(f, "or_c({},{})", l, r), + Terminal::OrB(ref l, ref r) => fmt_2(f, "or_b(", l, r, is_debug), + Terminal::OrD(ref l, ref r) => fmt_2(f, "or_d(", l, r, is_debug), + Terminal::OrC(ref l, ref r) => fmt_2(f, "or_c(", l, r, is_debug), Terminal::OrI(ref l, ref r) if l.node != Terminal::False && r.node != Terminal::False => { - write!(f, "or_i({},{})", l, r) - } - Terminal::Thresh(k, ref subs) => { - write!(f, "thresh({}", k)?; - for s in subs { - write!(f, ",{}", s)?; - } - f.write_str(")") - } - Terminal::Multi(k, ref keys) => { - write!(f, "multi({}", k)?; - for k in keys { - write!(f, ",{}", k)?; - } - f.write_str(")") - } - Terminal::MultiA(k, ref keys) => { - write!(f, "multi_a({}", k)?; - for k in keys { - write!(f, ",{}", k)?; - } - f.write_str(")") + fmt_2(f, "or_i(", l, r, is_debug) } + Terminal::Thresh(k, ref subs) => fmt_n(f, "thresh(", k, subs, is_debug), + Terminal::Multi(k, ref keys) => fmt_n(f, "multi(", k, keys, is_debug), + Terminal::MultiA(k, ref keys) => fmt_n(f, "multi_a(", k, keys, is_debug), // wrappers _ => { if let Some((ch, sub)) = self.wrap_char() { @@ -219,13 +107,13 @@ impl fmt::Display for Terminal { fmt::Write::write_char(f, ch)?; match sub.node.wrap_char() { None => { - fmt::Write::write_char(f, ':')?; + f.write_str(":")?; } // Add a ':' wrapper if there are other wrappers apart from c:pk_k() // tvc:pk_k() -> tv:pk() Some(('c', ms)) => match ms.node { Terminal::PkK(_) | Terminal::PkH(_) | Terminal::RawPkH(_) => { - fmt::Write::write_char(f, ':')? + f.write_str(":")?; } _ => {} }, @@ -240,6 +128,111 @@ impl fmt::Display for Terminal { } } +fn fmt_1( + f: &mut fmt::Formatter, + name: &str, + a: &D, + is_debug: bool, +) -> fmt::Result { + f.write_str(&name)?; + conditional_fmt(f, a, is_debug)?; + f.write_str(")") +} +fn fmt_2( + f: &mut fmt::Formatter, + name: &str, + a: &D, + b: &D, + is_debug: bool, +) -> fmt::Result { + f.write_str(&name)?; + conditional_fmt(f, a, is_debug)?; + f.write_str(",")?; + conditional_fmt(f, b, is_debug)?; + f.write_str(")") +} +fn fmt_n( + f: &mut fmt::Formatter, + name: &str, + first: usize, + list: &[D], + is_debug: bool, +) -> fmt::Result { + f.write_str(&name)?; + write!(f, "{}", first)?; + for el in list { + f.write_str(",")?; + conditional_fmt(f, el, is_debug)?; + } + f.write_str(")") +} +fn conditional_fmt( + f: &mut fmt::Formatter, + data: &D, + is_debug: bool, +) -> fmt::Result { + if is_debug { + fmt::Debug::fmt(data, f) + } else { + fmt::Display::fmt(data, f) + } +} + +impl fmt::Debug for Terminal { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str("[")?; + if let Ok(type_map) = types::Type::type_check(self) { + f.write_str(match type_map.corr.base { + types::Base::B => "B", + types::Base::K => "K", + types::Base::V => "V", + types::Base::W => "W", + })?; + fmt::Write::write_char(f, '/')?; + f.write_str(match type_map.corr.input { + types::Input::Zero => "z", + types::Input::One => "o", + types::Input::OneNonZero => "on", + types::Input::Any => "", + types::Input::AnyNonZero => "n", + })?; + if type_map.corr.dissatisfiable { + fmt::Write::write_char(f, 'd')?; + } + if type_map.corr.unit { + fmt::Write::write_char(f, 'u')?; + } + f.write_str(match type_map.mall.dissat { + types::Dissat::None => "f", + types::Dissat::Unique => "e", + types::Dissat::Unknown => "", + })?; + if type_map.mall.safe { + fmt::Write::write_char(f, 's')?; + } + if type_map.mall.non_malleable { + fmt::Write::write_char(f, 'm')?; + } + } else { + f.write_str("TYPECHECK FAILED")?; + } + f.write_str("]")?; + if let Some((ch, sub)) = self.wrap_char() { + fmt::Write::write_char(f, ch)?; + if sub.node.wrap_char().is_none() { + fmt::Write::write_char(f, ':')?; + } + write!(f, "{:?}", sub) + } else { + self.conditional_fmt(f, true) + } + } +} + +impl fmt::Display for Terminal { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.conditional_fmt(f, false) } +} + impl crate::expression::FromTree for Arc> { From 752acb378f3c7a6bc9b1f284f7060a1b43d841ab Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Tue, 5 Mar 2024 18:20:23 +0100 Subject: [PATCH 2/3] apply immediate dispatch in generic fn by refactoring out the function when the type is known, we avoid the duplication in the generic function. In the process write_char is replaced with write_str when the string is static because by looking at the impl it saves a utf8 str conversion from the char. --- src/miniscript/astelem.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index 7a39b071d..4c806a23e 100644 --- a/src/miniscript/astelem.rs +++ b/src/miniscript/astelem.rs @@ -180,15 +180,15 @@ fn conditional_fmt( impl fmt::Debug for Terminal { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str("[")?; - if let Ok(type_map) = types::Type::type_check(self) { + + fn fmt_type_map(f: &mut fmt::Formatter<'_>, type_map: types::Type) -> fmt::Result { f.write_str(match type_map.corr.base { types::Base::B => "B", types::Base::K => "K", types::Base::V => "V", types::Base::W => "W", })?; - fmt::Write::write_char(f, '/')?; + f.write_str("/")?; f.write_str(match type_map.corr.input { types::Input::Zero => "z", types::Input::One => "o", @@ -197,10 +197,10 @@ impl fmt::Debug for Terminal { types::Input::AnyNonZero => "n", })?; if type_map.corr.dissatisfiable { - fmt::Write::write_char(f, 'd')?; + f.write_str("d")?; } if type_map.corr.unit { - fmt::Write::write_char(f, 'u')?; + f.write_str("u")?; } f.write_str(match type_map.mall.dissat { types::Dissat::None => "f", @@ -208,11 +208,17 @@ impl fmt::Debug for Terminal { types::Dissat::Unknown => "", })?; if type_map.mall.safe { - fmt::Write::write_char(f, 's')?; + f.write_str("s")?; } if type_map.mall.non_malleable { - fmt::Write::write_char(f, 'm')?; + f.write_str("m")?; } + Ok(()) + } + + f.write_str("[")?; + if let Ok(type_map) = types::Type::type_check(self) { + fmt_type_map(f, type_map)?; } else { f.write_str("TYPECHECK FAILED")?; } @@ -220,7 +226,7 @@ impl fmt::Debug for Terminal { if let Some((ch, sub)) = self.wrap_char() { fmt::Write::write_char(f, ch)?; if sub.node.wrap_char().is_none() { - fmt::Write::write_char(f, ':')?; + f.write_str(":")?; } write!(f, "{:?}", sub) } else { From 395aab8131061f4c24b8d105132ad50e65275189 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Wed, 6 Mar 2024 09:56:03 +0100 Subject: [PATCH 3/3] unify debug/display impl for Terminal --- src/miniscript/astelem.rs | 24 ++++++++++-------------- src/miniscript/mod.rs | 16 ++++++++-------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index 4c806a23e..49bd2a84b 100644 --- a/src/miniscript/astelem.rs +++ b/src/miniscript/astelem.rs @@ -89,7 +89,7 @@ impl Terminal { if ch == 'c' { if let Terminal::PkK(ref pk) = sub.node { // alias: pk(K) = c:pk_k(K) - return write!(f, "pk({})", pk); + return fmt_1(f, "pk(", pk, is_debug); } else if let Terminal::RawPkH(ref pkh) = sub.node { // `RawPkH` is currently unsupported in the descriptor spec // alias: pkh(K) = c:pk_h(K) @@ -97,10 +97,10 @@ impl Terminal { // are not defined in the spec yet. These are prefixed with `expr` // in the descriptor string. // We do not support parsing these descriptors yet. - return write!(f, "expr_raw_pkh({})", pkh); + return fmt_1(f, "expr_raw_pkh(", pkh, is_debug); } else if let Terminal::PkH(ref pk) = sub.node { // alias: pkh(K) = c:pk_h(K) - return write!(f, "pkh({})", pk); + return fmt_1(f, "pkh(", pk, is_debug); } } @@ -119,7 +119,11 @@ impl Terminal { }, _ => {} }; - write!(f, "{}", sub) + if is_debug { + write!(f, "{:?}", sub) + } else { + write!(f, "{}", sub) + } } else { unreachable!(); } @@ -180,7 +184,6 @@ fn conditional_fmt( impl fmt::Debug for Terminal { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fn fmt_type_map(f: &mut fmt::Formatter<'_>, type_map: types::Type) -> fmt::Result { f.write_str(match type_map.corr.base { types::Base::B => "B", @@ -223,15 +226,8 @@ impl fmt::Debug for Terminal { f.write_str("TYPECHECK FAILED")?; } f.write_str("]")?; - if let Some((ch, sub)) = self.wrap_char() { - fmt::Write::write_char(f, ch)?; - if sub.node.wrap_char().is_none() { - f.write_str(":")?; - } - write!(f, "{:?}", sub) - } else { - self.conditional_fmt(f, true) - } + + self.conditional_fmt(f, true) } } diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 053fc6da1..03f41fc39 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -898,7 +898,7 @@ mod tests { phantom: PhantomData, })); let pkk_ms: Miniscript = Miniscript::from_ast(pk_node).unwrap(); - dummy_string_rtt(pkk_ms, "[B/onduesm]c:[K/onduesm]pk_k(\"\")", "pk()"); + dummy_string_rtt(pkk_ms, "[B/onduesm]pk(\"\")", "pk()"); let pkh_node = Terminal::Check(Arc::new(Miniscript { node: Terminal::PkH(String::from("")), @@ -908,7 +908,7 @@ mod tests { })); let pkh_ms: Miniscript = Miniscript::from_ast(pkh_node).unwrap(); - let expected_debug = "[B/nduesm]c:[K/nduesm]pk_h(\"\")"; + let expected_debug = "[B/nduesm]pkh(\"\")"; let expected_display = "pkh()"; assert_eq!(pkh_ms.ty.corr.base, types::Base::B); @@ -988,7 +988,7 @@ mod tests { string_rtt( script, - "[B/onduesm]c:[K/onduesm]pk_k(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", + "[B/onduesm]pk(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", "pk(028c28a97bf8298bc0d23d8c749452a32e694b65e30a9472a3954ab30fe5324caa)" ); @@ -996,7 +996,7 @@ mod tests { string_rtt( script, - "[B/onduesm]c:[K/onduesm]pk_k(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", + "[B/onduesm]pk(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", "pk(028c28a97bf8298bc0d23d8c749452a32e694b65e30a9472a3954ab30fe5324caa)" ); @@ -1004,7 +1004,7 @@ mod tests { string_rtt( script, - "[B/onufsm]t[V/onfsm]v[B/onduesm]c:[K/onduesm]pk_k(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", + "[B/onufsm]t[V/onfsm]v:[B/onduesm]pk(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", "tv:pk(028c28a97bf8298bc0d23d8c749452a32e694b65e30a9472a3954ab30fe5324caa)" ); @@ -1012,7 +1012,7 @@ mod tests { string_display_debug_test( script, - "[B/nduesm]c:[K/nduesm]pk_h(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", + "[B/nduesm]pkh(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", "pkh(028c28a97bf8298bc0d23d8c749452a32e694b65e30a9472a3954ab30fe5324caa)", ); @@ -1020,7 +1020,7 @@ mod tests { string_display_debug_test( script, - "[B/nduesm]c:[K/nduesm]pk_h(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", + "[B/nduesm]pkh(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", "pkh(028c28a97bf8298bc0d23d8c749452a32e694b65e30a9472a3954ab30fe5324caa)", ); @@ -1028,7 +1028,7 @@ mod tests { string_display_debug_test( script, - "[B/nufsm]t[V/nfsm]v[B/nduesm]c:[K/nduesm]pk_h(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", + "[B/nufsm]t[V/nfsm]v:[B/nduesm]pkh(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", "tv:pkh(028c28a97bf8298bc0d23d8c749452a32e694b65e30a9472a3954ab30fe5324caa)", ); }