From fec787c2815cf87847d6b06ff471456fdbc48674 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Fri, 3 Nov 2017 17:32:05 -0700 Subject: [PATCH] Optimize representation of Error without backtrace This change modifies Error to not store empty backtraces. This saves space and allows zero-sized errors without backtraces to be stored without any allocation. --- src/backtrace/internal.rs | 7 ++- src/backtrace/mod.rs | 7 ++- src/error.rs | 110 ++++++++++++++++++++++++-------------- 3 files changed, 78 insertions(+), 46 deletions(-) diff --git a/src/backtrace/internal.rs b/src/backtrace/internal.rs index 48d3b51..043ccab 100644 --- a/src/backtrace/internal.rs +++ b/src/backtrace/internal.rs @@ -18,6 +18,9 @@ struct MaybeResolved { unsafe impl Send for MaybeResolved {} unsafe impl Sync for MaybeResolved {} + +pub(super) const NONE: InternalBacktrace = InternalBacktrace { backtrace: None }; + impl InternalBacktrace { pub(super) fn new() -> InternalBacktrace { static ENABLED: AtomicUsize = ATOMIC_USIZE_INIT; @@ -45,10 +48,6 @@ impl InternalBacktrace { } } - pub(super) fn none() -> InternalBacktrace { - InternalBacktrace { backtrace: None } - } - pub(super) fn as_backtrace(&self) -> Option<&Backtrace> { let bt = match self.backtrace { Some(ref bt) => bt, diff --git a/src/backtrace/mod.rs b/src/backtrace/mod.rs index 3e896c2..1a19328 100644 --- a/src/backtrace/mod.rs +++ b/src/backtrace/mod.rs @@ -90,6 +90,9 @@ with_std! { internal: InternalBacktrace } + + pub(crate) const NONE: Backtrace = Backtrace { internal: internal::NONE }; + impl Backtrace { /// Construct a new backtrace. This will only create a real backtrace /// if the cate is compiled in std mode and the `RUST_BACKTRACE` @@ -98,8 +101,8 @@ with_std! { Backtrace { internal: InternalBacktrace::new() } } - pub(crate) fn none() -> Backtrace { - Backtrace { internal: InternalBacktrace::none() } + pub(crate) fn is_none(&self) -> bool { + self.internal.as_backtrace().is_none() } } diff --git a/src/error.rs b/src/error.rs index 9b343f6..03683be 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,10 +1,7 @@ use core::fmt::{self, Display, Debug}; -use core::mem; -use core::ptr; - use Fail; -use backtrace::Backtrace; +use backtrace::{self, Backtrace}; use context::Context; use compat::Compat; @@ -19,23 +16,59 @@ use compat::Compat; /// information, and can be downcast into the `Fail`ure that underlies it for /// more detailed inspection. pub struct Error { - pub(crate) inner: Box>, + pub(crate) inner: Box, } -pub(crate) struct Inner { +pub(crate) struct WithBacktrace { backtrace: Backtrace, - pub(crate) failure: F, + failure: F, +} + +// Unsafe because the `is_withbacktrace` method must be correct for memory safety. +pub(crate) unsafe trait FailOrWithBacktrace: Send + Sync + 'static { + fn fail_ref(&self) -> &Fail; + fn fail_mut(&mut self) -> &mut Fail; + fn backtrace(&self) -> &Backtrace; + fn is_withbacktrace(&self) -> bool; +} + +unsafe impl FailOrWithBacktrace for T { + fn fail_ref(&self) -> &Fail { self } + fn fail_mut(&mut self) -> &mut Fail { self } + fn backtrace(&self) -> &Backtrace { + static NONE: &'static Backtrace = &backtrace::NONE; + Fail::backtrace(self).unwrap_or(NONE) + } + fn is_withbacktrace(&self) -> bool { false } +} + +unsafe impl FailOrWithBacktrace for WithBacktrace { + fn fail_ref(&self) -> &Fail { &self.failure } + fn fail_mut(&mut self) -> &mut Fail { &mut self.failure } + fn backtrace(&self) -> &Backtrace { + &self.backtrace + } + fn is_withbacktrace(&self) -> bool { true } } impl From for Error { fn from(failure: F) -> Error { - let inner: Inner = { - let backtrace = if failure.backtrace().is_none() { - Backtrace::new() - } else { Backtrace::none() }; - Inner { failure, backtrace } + let inner = if failure.backtrace().is_some() { + Box::new(failure) + } else { + // Attempt to add a backtrace + let backtrace = Backtrace::new(); + if backtrace.is_none() { + Box::new(failure) as Box + } else { + Box::new(WithBacktrace { + backtrace, + failure, + }) + } }; - Error { inner: Box::new(inner) } + + Error { inner } } } @@ -44,7 +77,7 @@ impl Error { /// method on `Fail`, this does not return an Option. The Error type /// always has an underlying `Fail`ure. pub fn cause(&self) -> &Fail { - &self.inner.failure + self.inner.fail_ref() } /// Get a reference to the Backtrace for this Error. @@ -53,7 +86,7 @@ impl Error { /// be returned. Otherwise, the backtrace will have been constructed at /// the point that failure was cast into the Error type. pub fn backtrace(&self) -> &Backtrace { - self.inner.failure.backtrace().unwrap_or(&self.inner.backtrace) + self.inner.backtrace() } /// Provide context for this Error. @@ -89,21 +122,19 @@ impl Error { /// the case that the underlying error is of a different type, the /// original Error is returned. pub fn downcast(self) -> Result { - let ret: Option = self.downcast_ref().map(|fail| { - unsafe { - // drop the backtrace - let _ = ptr::read(&self.inner.backtrace as *const Backtrace); - // read out the fail type - ptr::read(fail as *const T) - } - }); - match ret { - Some(ret) => { - // forget self (backtrace is dropped, failure is moved - mem::forget(self); - Ok(ret) - } - _ => Err(self) + if self.downcast_ref::().is_some() { + Ok(unsafe { + let is_withbacktrace = self.inner.is_withbacktrace(); + let ptr: *mut FailOrWithBacktrace = Box::into_raw(self.inner); + + if is_withbacktrace { + Box::from_raw(ptr as *mut WithBacktrace).failure + } else { + *Box::from_raw(ptr as *mut T) + } + }) + } else { + Err(self) } } @@ -112,7 +143,7 @@ impl Error { /// /// If the underlying error is not of type `T`, this will return `None`. pub fn downcast_ref(&self) -> Option<&T> { - self.inner.failure.downcast_ref() + self.inner.fail_ref().downcast_ref::() } /// Attempt to downcast this Error to a particular `Fail` type by @@ -120,25 +151,24 @@ impl Error { /// /// If the underlying error is not of type `T`, this will return `None`. pub fn downcast_mut(&mut self) -> Option<&mut T> { - self.inner.failure.downcast_mut() + self.inner.fail_mut().downcast_mut::() } } impl Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - Display::fmt(&self.inner.failure, f) - } -} - -impl Debug for Inner { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "Error {{ failure: {:?} }}\n\n{:?}", &self.failure, &self.backtrace) + Display::fmt(&self.cause(), f) } } impl Debug for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{:?}", &self.inner) + write!(f, "Error {{ failure: {:?} }}\n\n", self.inner.fail_ref())?; + let backtrace = self.inner.backtrace(); + if !backtrace.is_none() { + write!(f, "{:?}", backtrace)?; + } + Ok(()) } }