From f3020b81d7bd2c26895ac0174f91c4cbb47a4dfe Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 23 Sep 2017 11:17:12 -0700 Subject: [PATCH] Shrink the size of all `Error` types This commit improves the in-memory size of `Error` from 7 pointers to 1 pointer. Errors are in general relatively rare in applications and having a huge error type ends up generating lots of instructions for moves and such, so this PR optimizes for size and passing around errors rather than creating errors. --- CHANGELOG.md | 1 + examples/chain_err.rs | 6 +++--- examples/size.rs | 30 ------------------------------ src/error_chain.rs | 43 ++++++++++++++++++------------------------- src/lib.rs | 10 +++++----- tests/tests.rs | 14 +++++++++----- 6 files changed, 36 insertions(+), 68 deletions(-) delete mode 100644 examples/size.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index f60c765f3..d3ec66261 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased - [Remove `impl Deref for Error`](https://github.com/rust-lang-nursery/error-chain/pull/192) +- [Shrink the size of `Error` to a pointer](https://github.com/rust-lang-nursery/error-chain/pull/225) # 0.11.0 diff --git a/examples/chain_err.rs b/examples/chain_err.rs index bd8effdaf..61b170501 100644 --- a/examples/chain_err.rs +++ b/examples/chain_err.rs @@ -53,11 +53,11 @@ fn load_config(rel_path: &str) -> Result<()> { /// Launch the service. fn launch(rel_path: &str) -> Result<()> { - load_config(rel_path).map_err(|e| match e { - e @ Error(ErrorKind::ConfigLoad(_), _) => { + load_config(rel_path).map_err(|e| match *e.kind() { + ErrorKind::ConfigLoad(_) => { e.chain_err(|| LaunchStage::ConfigLoad) } - e => e.chain_err(|| "Unknown failure"), + _ => e.chain_err(|| "Unknown failure"), }) } diff --git a/examples/size.rs b/examples/size.rs deleted file mode 100644 index 3e180684e..000000000 --- a/examples/size.rs +++ /dev/null @@ -1,30 +0,0 @@ -#[macro_use] -extern crate error_chain; - -use std::mem::{size_of, size_of_val}; - -error_chain! { - errors { - AVariant - Another - } -} - -fn main() { - println!("Memory usage in bytes"); - println!("---------------------"); - println!("Result<()>: {}", size_of::>()); - println!(" (): {}", size_of::<()>()); - println!(" Error: {}", size_of::()); - println!(" ErrorKind: {}", size_of::()); - let msg = ErrorKind::Msg("test".into()); - println!(" ErrorKind::Msg: {}", size_of_val(&msg)); - println!(" String: {}", size_of::()); - println!(" State: {}", size_of::()); - let state = error_chain::State { - next_error: None, - backtrace: error_chain::InternalBacktrace::new(), - }; - println!(" State.next_error: {}", size_of_val(&state.next_error)); - println!(" State.backtrace: {}", size_of_val(&state.backtrace)); -} diff --git a/src/error_chain.rs b/src/error_chain.rs index 9b95a77d3..db83e1ec8 100644 --- a/src/error_chain.rs +++ b/src/error_chain.rs @@ -65,20 +65,13 @@ macro_rules! impl_error_chain_processed { /// - a backtrace, generated when the error is created. /// - an error chain, used for the implementation of `Error::cause()`. #[derive(Debug)] - pub struct $error_name( - // The members must be `pub` for `links`. - /// The kind of the error. - pub $error_kind_name, - /// Contains the error chain and the backtrace. - #[doc(hidden)] - pub $crate::State, - ); + pub struct $error_name(pub Box<($error_kind_name, $crate::State)>); impl $crate::ChainedError for $error_name { type ErrorKind = $error_kind_name; fn new(kind: $error_kind_name, state: $crate::State) -> $error_name { - $error_name(kind, state) + $error_name(Box::new((kind, state))) } fn from_kind(kind: Self::ErrorKind) -> Self { @@ -120,10 +113,10 @@ macro_rules! impl_error_chain_processed { impl $error_name { /// Constructs an error from a kind, and generates a backtrace. pub fn from_kind(kind: $error_kind_name) -> $error_name { - $error_name( + $error_name(Box::new(( kind, $crate::State::default(), - ) + ))) } /// Constructs a chained error from another error and a kind, and generates a backtrace. @@ -140,15 +133,15 @@ macro_rules! impl_error_chain_processed { -> $error_name where K: Into<$error_kind_name> { - $error_name( + $error_name(Box::new(( kind.into(), $crate::State::new::<$error_name>(error, ), - ) + ))) } /// Returns the kind of the error. pub fn kind(&self) -> &$error_kind_name { - &self.0 + &(self.0).0 } /// Iterates over the error chain. @@ -158,7 +151,7 @@ macro_rules! impl_error_chain_processed { /// Returns the backtrace associated with this error. pub fn backtrace(&self) -> Option<&$crate::Backtrace> { - self.1.backtrace() + (self.0).1.backtrace() } /// Extends the error chain with a new entry. @@ -170,7 +163,7 @@ macro_rules! impl_error_chain_processed { /// A short description of the error. /// This method is identical to [`Error::description()`](https://doc.rust-lang.org/nightly/std/error/trait.Error.html#tymethod.description) pub fn description(&self) -> &str { - self.0.description() + (self.0).0.description() } } @@ -181,10 +174,10 @@ macro_rules! impl_error_chain_processed { #[allow(unknown_lints, unused_doc_comment)] fn cause(&self) -> Option<&::std::error::Error> { - match self.1.next_error { + match (self.0).1.next_error { Some(ref c) => Some(&**c), None => { - match self.0 { + match (self.0).0 { $( $(#[$meta_foreign_links])* $error_kind_name::$foreign_link_variant(ref foreign_err) => { @@ -200,7 +193,7 @@ macro_rules! impl_error_chain_processed { impl ::std::fmt::Display for $error_name { fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { - ::std::fmt::Display::fmt(&self.0, f) + ::std::fmt::Display::fmt(&(self.0).0, f) } } @@ -208,10 +201,11 @@ macro_rules! impl_error_chain_processed { $(#[$meta_links])* impl From<$link_error_path> for $error_name { fn from(e: $link_error_path) -> Self { - $error_name( + let e = *e.0; + $error_name(Box::new(( $error_kind_name::$link_variant(e.0), e.1, - ) + ))) } } ) * @@ -245,7 +239,6 @@ macro_rules! impl_error_chain_processed { } } - // The ErrorKind type // -------------- @@ -303,7 +296,7 @@ macro_rules! impl_error_chain_processed { impl From<$error_name> for $error_kind_name { fn from(e: $error_name) -> Self { - e.0 + (e.0).0 } } @@ -425,13 +418,13 @@ macro_rules! impl_extract_backtrace { fn extract_backtrace(e: &(::std::error::Error + Send + 'static)) -> Option<$crate::InternalBacktrace> { if let Some(e) = e.downcast_ref::<$error_name>() { - return Some(e.1.backtrace.clone()); + return Some((e.0).1.backtrace.clone()); } $( $( #[$meta_links] )* { if let Some(e) = e.downcast_ref::<$link_error_path>() { - return Some(e.1.backtrace.clone()); + return Some((e.0).1.backtrace.clone()); } } ) * diff --git a/src/lib.rs b/src/lib.rs index ac185c643..0b6fd09b0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -345,9 +345,9 @@ //! } //! } //! -//! match Error::from("error!") { -//! Error(ErrorKind::InvalidToolchainName(_), _) => { } -//! Error(ErrorKind::Msg(_), _) => { } +//! match *Error::from("error!").kind() { +//! ErrorKind::InvalidToolchainName(_) => { } +//! ErrorKind::Msg(_) => { } //! _ => { } //! } //! # } @@ -377,8 +377,8 @@ //! //! //! # fn main() { -//! match app::Error::from("error!") { -//! app::Error(app::ErrorKind::Utils(utils::ErrorKind::BadStuff), _) => { } +//! match *app::Error::from("error!").kind() { +//! app::ErrorKind::Utils(utils::ErrorKind::BadStuff) => { } //! _ => { } //! } //! # } diff --git a/tests/tests.rs b/tests/tests.rs index 0e38e5448..005e191e1 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -255,7 +255,7 @@ fn error_chain_err() { let base = Error::from(ErrorKind::Test); let ext = base.chain_err(|| "Test passes"); - if let Error(ErrorKind::Msg(_), _) = ext { + if let ErrorKind::Msg(_) = *ext.kind() { // pass } else { panic!("The error should be wrapped. {:?}", ext); @@ -484,8 +484,8 @@ fn error_patterns() { } // Tuples look nice when matching errors - match Error::from("Test") { - Error(ErrorKind::Msg(_), _) => {}, + match *Error::from("Test").kind() { + ErrorKind::Msg(_) => {}, _ => {}, } } @@ -500,8 +500,12 @@ fn result_match() { match ok() { Ok(()) => {}, - Err(Error(ErrorKind::Msg(_), _)) => {}, - Err(..) => {}, + Err(e) => { + match *e.kind() { + ErrorKind::Msg(_) => {} + _ => {} + } + } } }