From 0c411db3fbf950cedccf113e634bfb8e1b89c314 Mon Sep 17 00:00:00 2001 From: Marko Bencun Date: Sun, 30 Jul 2023 11:34:33 +0200 Subject: [PATCH] reduce binary bloat by removing generic param from type_check The binary sizes are increased a lot by the use of generics. In one case, there is a lot of needless bloat by using a generic type param for the callback argument in the type_check function of the Property trait. cargo build --example parse --release && du -sb target/release/examples/parse Results in a binary size of 1784152 bytes. With this commit, the binary size is decreased by 12288 bytes, which is significant for embedded/no_std use. The sizes above were achieved by adding this to Cargo.toml: ``` [profile.release] strip = true panic = "abort" opt-level = 's' codegen-units = 1 lto = true ``` Though the difference is roughly the same also without the optimizations. As an additional benefit, code clarity is improved as the child arg was unused in some of the trait implementations. --- src/miniscript/astelem.rs | 2 +- src/miniscript/decode.rs | 20 ++++------ src/miniscript/mod.rs | 6 +-- src/miniscript/types/extra_props.rs | 18 ++++++--- src/miniscript/types/mod.rs | 61 +++++++++++++++++++---------- src/policy/compiler.rs | 24 ++++++------ 6 files changed, 78 insertions(+), 53 deletions(-) diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index c7d403679..b272830e1 100644 --- a/src/miniscript/astelem.rs +++ b/src/miniscript/astelem.rs @@ -47,7 +47,7 @@ impl Terminal { 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, |_| None) { + 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", diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs index d5fa907a6..5dc532f27 100644 --- a/src/miniscript/decode.rs +++ b/src/miniscript/decode.rs @@ -25,10 +25,6 @@ use crate::prelude::*; use crate::Descriptor; use crate::{bitcoin, hash256, AbsLockTime, Error, Miniscript, MiniscriptKey, ToPublicKey}; -fn return_none(_: usize) -> Option { - None -} - /// Trait for parsing keys from byte slices pub trait ParseableKey: Sized + ToPublicKey + private::Sealed { /// Parse a key from slice @@ -223,8 +219,8 @@ impl TerminalStack { ///reduce, type check and push a 0-arg node fn reduce0(&mut self, ms: Terminal) -> Result<(), Error> { - let ty = Type::type_check(&ms, return_none)?; - let ext = ExtData::type_check(&ms, return_none)?; + let ty = Type::type_check(&ms)?; + let ext = ExtData::type_check(&ms)?; let ms = Miniscript { node: ms, ty, @@ -244,8 +240,8 @@ impl TerminalStack { let top = self.pop().unwrap(); let wrapped_ms = wrap(Arc::new(top)); - let ty = Type::type_check(&wrapped_ms, return_none)?; - let ext = ExtData::type_check(&wrapped_ms, return_none)?; + let ty = Type::type_check(&wrapped_ms)?; + let ext = ExtData::type_check(&wrapped_ms)?; let ms = Miniscript { node: wrapped_ms, ty, @@ -267,8 +263,8 @@ impl TerminalStack { let wrapped_ms = wrap(Arc::new(left), Arc::new(right)); - let ty = Type::type_check(&wrapped_ms, return_none)?; - let ext = ExtData::type_check(&wrapped_ms, return_none)?; + let ty = Type::type_check(&wrapped_ms)?; + let ext = ExtData::type_check(&wrapped_ms)?; let ms = Miniscript { node: wrapped_ms, ty, @@ -556,8 +552,8 @@ pub fn parse( let c = term.pop().unwrap(); let wrapped_ms = Terminal::AndOr(Arc::new(a), Arc::new(c), Arc::new(b)); - let ty = Type::type_check(&wrapped_ms, return_none)?; - let ext = ExtData::type_check(&wrapped_ms, return_none)?; + let ty = Type::type_check(&wrapped_ms)?; + let ext = ExtData::type_check(&wrapped_ms)?; term.0.push(Miniscript { node: wrapped_ms, diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index ed607977c..cd9c912ea 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -68,8 +68,8 @@ impl Miniscript { /// Display code of type_check. pub fn from_ast(t: Terminal) -> Result, Error> { let res = Miniscript { - ty: Type::type_check(&t, |_| None)?, - ext: ExtData::type_check(&t, |_| None)?, + ty: Type::type_check(&t)?, + ext: ExtData::type_check(&t)?, node: t, phantom: PhantomData, }; @@ -269,7 +269,7 @@ impl Miniscript { let top = decode::parse(&mut iter)?; Ctx::check_global_validity(&top)?; - let type_check = types::Type::type_check(&top.node, |_| None)?; + let type_check = types::Type::type_check(&top.node)?; if type_check.corr.base != types::Base::B { return Err(Error::NonTopLevel(format!("{:?}", top))); }; diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs index 3f12e5a88..71f3a2714 100644 --- a/src/miniscript/types/extra_props.rs +++ b/src/miniscript/types/extra_props.rs @@ -888,14 +888,22 @@ impl Property for ExtData { }) } + fn type_check_with_child( + _fragment: &Terminal, + mut _child: C, + ) -> Result> + where + C: FnMut(usize) -> Self, + Pk: MiniscriptKey, + Ctx: ScriptContext, + { + unreachable!() + } + /// Compute the type of a fragment assuming all the children of /// Miniscript have been computed already. - fn type_check( - fragment: &Terminal, - _child: C, - ) -> Result> + fn type_check(fragment: &Terminal) -> Result> where - C: FnMut(usize) -> Option, Ctx: ScriptContext, Pk: MiniscriptKey, { diff --git a/src/miniscript/types/mod.rs b/src/miniscript/types/mod.rs index 35a29605e..e4d8120fb 100644 --- a/src/miniscript/types/mod.rs +++ b/src/miniscript/types/mod.rs @@ -20,12 +20,6 @@ pub use self::malleability::{Dissat, Malleability}; use super::ScriptContext; use crate::{MiniscriptKey, Terminal}; -/// None-returning function to help type inference when we need a -/// closure that simply returns `None` -fn return_none(_: usize) -> Option { - None -} - /// Detailed type of a typechecker error #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] pub enum ErrorKind { @@ -374,20 +368,15 @@ pub trait Property: Sized { /// Compute the type of a fragment, given a function to look up /// the types of its children, if available and relevant for the /// given fragment - fn type_check( - fragment: &Terminal, - mut child: C, + fn type_check_common<'a, Pk, Ctx, C>( + fragment: &'a Terminal, + mut get_child: C, ) -> Result> where - C: FnMut(usize) -> Option, + C: FnMut(&'a Terminal, usize) -> Result>, Pk: MiniscriptKey, Ctx: ScriptContext, { - let mut get_child = |sub, n| { - child(n) - .map(Ok) - .unwrap_or_else(|| Self::type_check(sub, return_none)) - }; let wrap_err = |result: Result| { result.map_err(|kind| Error { fragment: fragment.clone(), @@ -523,6 +512,30 @@ pub trait Property: Sized { } ret } + + /// Compute the type of a fragment, given a function to look up + /// the types of its children. + fn type_check_with_child( + fragment: &Terminal, + mut child: C, + ) -> Result> + where + C: FnMut(usize) -> Self, + Pk: MiniscriptKey, + Ctx: ScriptContext, + { + let get_child = |_sub, n| Ok(child(n)); + Self::type_check_common(fragment, get_child) + } + + /// Compute the type of a fragment. + fn type_check(fragment: &Terminal) -> Result> + where + Pk: MiniscriptKey, + Ctx: ScriptContext, + { + Self::type_check_common(fragment, |sub, _n| Self::type_check(sub)) + } } impl Property for Type { @@ -767,14 +780,22 @@ impl Property for Type { }) } + fn type_check_with_child( + _fragment: &Terminal, + mut _child: C, + ) -> Result> + where + C: FnMut(usize) -> Self, + Pk: MiniscriptKey, + Ctx: ScriptContext, + { + unreachable!() + } + /// Compute the type of a fragment assuming all the children of /// Miniscript have been computed already. - fn type_check( - fragment: &Terminal, - _child: C, - ) -> Result> + fn type_check(fragment: &Terminal) -> Result> where - C: FnMut(usize) -> Option, Pk: MiniscriptKey, Ctx: ScriptContext, { diff --git a/src/policy/compiler.rs b/src/policy/compiler.rs index c3c13ec62..de117d32e 100644 --- a/src/policy/compiler.rs +++ b/src/policy/compiler.rs @@ -480,7 +480,7 @@ impl AstElemExt { impl AstElemExt { fn terminal(ast: Terminal) -> AstElemExt { AstElemExt { - comp_ext_data: CompilerExtData::type_check(&ast, |_| None).unwrap(), + comp_ext_data: CompilerExtData::type_check(&ast).unwrap(), ms: Arc::new(Miniscript::from_ast(ast).expect("Terminal creation must always succeed")), } } @@ -491,15 +491,15 @@ impl AstElemExt { r: &AstElemExt, ) -> Result, types::Error> { let lookup_ext = |n| match n { - 0 => Some(l.comp_ext_data), - 1 => Some(r.comp_ext_data), + 0 => l.comp_ext_data, + 1 => r.comp_ext_data, _ => unreachable!(), }; //Types and ExtData are already cached and stored in children. So, we can //type_check without cache. For Compiler extra data, we supply a cache. - let ty = types::Type::type_check(&ast, |_| None)?; - let ext = types::ExtData::type_check(&ast, |_| None)?; - let comp_ext_data = CompilerExtData::type_check(&ast, lookup_ext)?; + let ty = types::Type::type_check(&ast)?; + let ext = types::ExtData::type_check(&ast)?; + let comp_ext_data = CompilerExtData::type_check_with_child(&ast, lookup_ext)?; Ok(AstElemExt { ms: Arc::new(Miniscript::from_components_unchecked(ast, ty, ext)), comp_ext_data, @@ -513,16 +513,16 @@ impl AstElemExt { c: &AstElemExt, ) -> Result, types::Error> { let lookup_ext = |n| match n { - 0 => Some(a.comp_ext_data), - 1 => Some(b.comp_ext_data), - 2 => Some(c.comp_ext_data), + 0 => a.comp_ext_data, + 1 => b.comp_ext_data, + 2 => c.comp_ext_data, _ => unreachable!(), }; //Types and ExtData are already cached and stored in children. So, we can //type_check without cache. For Compiler extra data, we supply a cache. - let ty = types::Type::type_check(&ast, |_| None)?; - let ext = types::ExtData::type_check(&ast, |_| None)?; - let comp_ext_data = CompilerExtData::type_check(&ast, lookup_ext)?; + let ty = types::Type::type_check(&ast)?; + let ext = types::ExtData::type_check(&ast)?; + let comp_ext_data = CompilerExtData::type_check_with_child(&ast, lookup_ext)?; Ok(AstElemExt { ms: Arc::new(Miniscript::from_components_unchecked(ast, ty, ext)), comp_ext_data,