apache / apache/arrow-adbc

rust/core/error: Make `Error` struct be boxed so stack frames aren't very large

Open
#4,667 5 comments 1 reaction 0 assignees View on GitHub
Type: enhancement
Dominant language
C#
Stars
627
Forks
217
Avg merge
17h
Merged PRs (30d)
57

Description

## Problem

Currently the `adbc_core::Error` struct contains all the fields (message, status, vendor code etc), and it is directly embedded in `adbc_core::Result`. That means regardless of `sizeof(T)`, `adbc_core::Result` is always at least `sizeof(Error)`, which is large.

This makes applications pay a lot of stack frame space for the cold error handling path since every `Result` is big.

### In concrete terms

On my ARM Mac:

Image

So currently the error is 64 bytes large whereas a pointer is only 8.

## Solution

Change it to something like

```rust
pub struct Error(Box);

pub struct ErrorInner {
message: String,
// ...
}

impl Error {
pub fn message(&self) -> &str {
&self.0.message
}

// ....
}
```

Now `sizeof(Error)` is effectively the size of a thin pointer. The downside is that this requires an extra allocation and dereference on every error, but I assume that's okay since errors are (hopefully) in the cold path.

Note that this will be a massive breaking change since currently all the fields in `Error` are `pub` so virtually everyone doing error handling based on those public fields will need to change their code to use accessors instead. We'll also need a constructor and people will need to move away from manually initializing the struct.

## The current workaround

Write a wrapper to all methods of the driver manager that returns a custom error type that does the wrapping for you.

The Rust `driverbase` of ADBC Foundry already does that: https://github.com/adbc-drivers/driverbase-rs/blob/fe2738281cae9a2a92c59cc8c4d0822395187d04/driverbase/src/error.rs#L19

## Edits

EDIT1: Added MacOS size comparison

Contributor guide

Open the contributing guide

Research direction

Start with the adbc_core::Error and Result definitions, then compare the existing error API with driverbase/src/error.rs referenced in the issue. Done means Error uses boxed storage, exposes the needed construction and access patterns, and the affected public-field users can migrate without keeping the large inline error in every Result.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.