Skip to content

Commit 6478d51

Browse files
robert3005claude
authored andcommitted
Put VortexError's payload behind one Arc
VortexError was 56 bytes, so every VortexResult was too, whatever it carried. A result that large is returned through memory: the caller reserves a stack slot, passes its address, and reads the outcome back. That is the path for nearly every call in the codebase, almost all of which succeed: 3158 functions return VortexResult<()> and 684 return VortexResult<ArrayRef>. The kind, message, source and backtrace now live in one shared allocation, and VortexError is a single pointer: VortexResult<T> for T before after () 56 8 bool, usize, ArrayRef 56 16 Option<ArrayRef>, Vec 56 24 Results of 16 bytes or less come back in registers on x86-64 and aarch64. Const assertions pin the error and VortexResult<()> to one word. Constructing an error costs no more than before. It already allocated every time, for the Arc around its backtrace; the backtrace now sits inline in the payload, so that allocation is the payload's. Wrapping a foreign error drops from three allocations to two, because its source is now boxed once instead of boxed and then copied into an Arc. Cloning stays a reference-count bump. with_context writes into the payload when it holds the only reference, which is the usual case while an error propagates. On an error that has been cloned, it builds a new payload that points back at the shared one for its source and backtrace, so the other clones are unchanged and nothing is lost. Signed-off-by: Robert Kruszewski <github@robertk.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S7EtMqoZ91rrCCh9vS2yTm
1 parent 4442700 commit 6478d51

2 files changed

Lines changed: 104 additions & 31 deletions

File tree

‎vortex-error/src/lib.rs‎

Lines changed: 76 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -132,26 +132,42 @@ impl VortexErrorKind {
132132
///
133133
/// An error is a [`VortexErrorKind`], a message, an optional underlying error, and a backtrace
134134
/// captured at construction. Cloning is cheap: the payload is shared, never copied.
135+
///
136+
/// The payload sits behind one pointer, so a `VortexResult<T>` costs the same as `T` plus at
137+
/// most a word on the success path. Most results fit in two registers and are returned in them,
138+
/// rather than through a stack slot the caller must reserve and read back. Constructing an error
139+
/// is no more expensive for it: the payload's allocation is the one the backtrace needed anyway.
135140
#[derive(Clone)]
136-
pub struct VortexError {
141+
pub struct VortexError(Arc<ErrorInner>);
142+
143+
const _: () = assert!(size_of::<VortexError>() == size_of::<usize>());
144+
const _: () = assert!(size_of::<VortexResult<()>>() == size_of::<usize>());
145+
146+
struct ErrorInner {
137147
kind: VortexErrorKind,
138-
/// `None` defers the message to `source`, keeping the `?` conversion path allocation-free.
148+
/// `None` defers the message to the source, keeping the `?` conversion path allocation-free.
139149
message: Option<ErrString>,
140-
source: Option<Arc<dyn Error + Send + Sync + 'static>>,
141-
backtrace: Arc<Backtrace>,
150+
origin: Origin,
142151
}
143152

144-
const _: () = assert!(size_of::<VortexError>() <= 56);
153+
/// Where an error's source and backtrace come from.
154+
enum Origin {
155+
/// Captured when this error was created.
156+
Own {
157+
source: Option<Box<dyn Error + Send + Sync + 'static>>,
158+
backtrace: Backtrace,
159+
},
160+
/// Shared with the error this one adds context to.
161+
///
162+
/// Context is normally added in place. This is only for an error that has been cloned, whose
163+
/// payload the other clones still see.
164+
Context(VortexError),
165+
}
145166

146167
impl VortexError {
147168
/// Creates an error of the given kind carrying `message`.
148169
pub fn new<T: Into<ErrString>>(kind: VortexErrorKind, message: T) -> Self {
149-
Self {
150-
kind,
151-
message: Some(message.into()),
152-
source: None,
153-
backtrace: Arc::new(Backtrace::capture()),
154-
}
170+
Self::from_parts(kind, Some(message.into()), None)
155171
}
156172

157173
/// Wraps an underlying error as a Vortex error of the given kind.
@@ -162,12 +178,7 @@ impl VortexError {
162178
where
163179
E: Into<Box<dyn Error + Send + Sync + 'static>>,
164180
{
165-
Self {
166-
kind,
167-
message: None,
168-
source: Some(Arc::from(source.into())),
169-
backtrace: Arc::new(Backtrace::capture()),
170-
}
181+
Self::from_parts(kind, None, Some(source.into()))
171182
}
172183

173184
/// Wraps an underlying error that does not fit any more specific [`VortexErrorKind`].
@@ -178,43 +189,78 @@ impl VortexError {
178189
Self::wrap(VortexErrorKind::Other, source)
179190
}
180191

192+
fn from_parts(
193+
kind: VortexErrorKind,
194+
message: Option<ErrString>,
195+
source: Option<Box<dyn Error + Send + Sync + 'static>>,
196+
) -> Self {
197+
Self(Arc::new(ErrorInner {
198+
kind,
199+
message,
200+
origin: Origin::Own {
201+
source,
202+
backtrace: Backtrace::capture(),
203+
},
204+
}))
205+
}
206+
181207
/// The classification of this error.
182208
pub fn kind(&self) -> VortexErrorKind {
183-
self.kind
209+
self.0.kind
184210
}
185211

186212
/// Adds additional context to an error, preserving its kind, source and backtrace.
187213
pub fn with_context<T: Into<ErrString>>(mut self, msg: T) -> Self {
188214
let msg: ErrString = msg.into();
189215
// Build the combined string directly: `msg` has already been through the
190216
// `VORTEX_PANIC_ON_ERR` check and must not trip it a second time.
191-
self.message = Some(ErrString(Cow::Owned(format!(
192-
"{msg}:\n {}",
193-
self.message_body()
194-
))));
195-
self
217+
let message = ErrString(Cow::Owned(format!("{msg}:\n {}", self.message_body())));
218+
if let Some(inner) = Arc::get_mut(&mut self.0) {
219+
inner.message = Some(message);
220+
return self;
221+
}
222+
Self(Arc::new(ErrorInner {
223+
kind: self.kind(),
224+
message: Some(message),
225+
origin: Origin::Context(self),
226+
}))
196227
}
197228

198229
/// The error message, falling back to the underlying error when none was provided.
199230
fn message_body(&self) -> Cow<'_, str> {
200-
match (&self.message, &self.source) {
231+
match (&self.0.message, self.source_ref()) {
201232
(Some(message), _) => Cow::Borrowed(message.as_ref()),
202233
(None, Some(source)) => Cow::Owned(source.to_string()),
203234
(None, None) => Cow::Borrowed(""),
204235
}
205236
}
237+
238+
fn source_ref(&self) -> Option<&(dyn Error + Send + Sync + 'static)> {
239+
match &self.0.origin {
240+
Origin::Own { source, .. } => source.as_deref(),
241+
Origin::Context(error) => error.source_ref(),
242+
}
243+
}
244+
245+
fn backtrace(&self) -> &Backtrace {
246+
match &self.0.origin {
247+
Origin::Own { backtrace, .. } => backtrace,
248+
Origin::Context(error) => error.backtrace(),
249+
}
250+
}
206251
}
207252

208253
impl Display for VortexError {
209254
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
210-
f.write_str(self.kind.prefix())?;
211-
match (&self.message, &self.source) {
255+
f.write_str(self.kind().prefix())?;
256+
match (&self.0.message, self.source_ref()) {
212257
(Some(message), _) => Display::fmt(message, f)?,
213258
(None, Some(source)) => Display::fmt(source, f)?,
214259
(None, None) => {}
215260
}
216-
if self.backtrace.status() == BacktraceStatus::Captured {
217-
write!(f, "\nBacktrace:\n{}", self.backtrace)?;
261+
let backtrace = self.backtrace();
262+
if backtrace.status() == BacktraceStatus::Captured {
263+
write!(f, "\nBacktrace:\n{backtrace}")?;
218264
}
219265
Ok(())
220266
}
@@ -228,9 +274,8 @@ impl Debug for VortexError {
228274

229275
impl Error for VortexError {
230276
fn source(&self) -> Option<&(dyn Error + 'static)> {
231-
self.source
232-
.as_ref()
233-
.map(|source| source.as_ref() as &(dyn Error + 'static))
277+
self.source_ref()
278+
.map(|source| source as &(dyn Error + 'static))
234279
}
235280
}
236281

‎vortex-error/tests/kind.rs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,34 @@ fn test_clone_and_shared_round_trip() {
141141
assert_eq!(borrowed.to_string(), owned.to_string());
142142
}
143143

144+
#[test]
145+
fn test_context_on_a_clone_leaves_the_other_clone_alone() {
146+
let err = VortexError::from(io::Error::other("disk on fire"));
147+
let clone = err.clone();
148+
149+
// The payload is shared, so this context cannot be written into it in place.
150+
let with_context = clone.with_context("while reading");
151+
152+
let original = err.to_string();
153+
assert!(
154+
original.starts_with("IO error: disk on fire") && !original.contains("while reading"),
155+
"{original}"
156+
);
157+
let display = with_context.to_string();
158+
assert!(
159+
display.starts_with("IO error: while reading:\n disk on fire"),
160+
"{display}"
161+
);
162+
assert_eq!(with_context.kind(), VortexErrorKind::Io);
163+
assert!(
164+
with_context
165+
.source()
166+
.and_then(|source| source.downcast_ref::<io::Error>())
167+
.is_some(),
168+
"context on a shared error must still expose the underlying error"
169+
);
170+
}
171+
144172
#[test]
145173
fn test_arrow_error_is_classified_by_variant() {
146174
use arrow_schema::ArrowError;

0 commit comments

Comments
 (0)