Repository navigation
Reclassify and simplify error types - #10080
robert3005 wants to merge 2 commits into
Conversation
e45d46e to
824bbb4
Compare
Merging this PR will degrade performance by 0.53%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | decode_bool_nullable[10000_2_alternating_mostly_valid] |
368.2 µs | 470 µs | -21.66% |
| ❌ | Simulation | decode_bool_nullable[10000_10_mostly_true_mostly_valid] |
89.9 µs | 111.1 µs | -19.13% |
| ❌ | Simulation | decode_bool_nullable[10000_10_alternating_mostly_null] |
90.8 µs | 111.8 µs | -18.77% |
| ❌ | Simulation | decode_bool_nullable[10000_10_alternating_half_valid] |
109.1 µs | 127.6 µs | -14.5% |
| ❌ | Simulation | decode_bool_nullable[10000_10_alternating_mostly_valid] |
120.2 µs | 140 µs | -14.09% |
| ❌ | Simulation | decode_bool_nullable[10000_10_mostly_true_half_valid] |
112 µs | 129.9 µs | -13.81% |
| ❌ | Simulation | decode_bool_nullable[10000_10_mostly_true_mostly_null] |
113 µs | 129.6 µs | -12.78% |
| ❌ | Simulation | from_bool_slice[1024] |
3.4 µs | 3.8 µs | -10.94% |
| ❌ | Simulation | split[(65536, Microseconds)] |
2.1 ms | 2.4 ms | -10.37% |
| ⚡ | Simulation | validate_struct8 |
383.6 µs | 287.1 µs | +33.62% |
| ⚡ |
Simulation | random_i128[0.01] |
8 µs | 6.7 µs | +19.6% |
| ⚡ | Simulation | sum_i64 |
225.1 µs | 191.8 µs | +17.39% |
| ⚡ | Simulation | sum_v2_i64 |
224.8 µs | 193.2 µs | +16.37% |
| ⚡ | Simulation | all_valid_exclusive[65536] |
3.5 ms | 3.1 ms | +14.45% |
| ⚡ | Simulation | all_valid_exclusive[4096] |
229.7 µs | 201.1 µs | +14.2% |
| ⚡ | Simulation | fsl_i32[16, 0.01] |
27 µs | 23.7 µs | +13.82% |
| ⚡ | WallTime | compare_u8_neon |
2 µs | 1.8 µs | +11.98% |
| ⚡ | Simulation | try_new_struct8 |
923.9 µs | 839.1 µs | +10.11% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/affectionate-keller-4j7w5g (74a7141) with develop (731a231)
Footnotes
-
342 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
26 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them. ↩
824bbb4 to
a36c01c
Compare
2ee2c82 to
7f17feb
Compare
Polar Signals Profiling ResultsLatest Run
Powered by Polar Signals Cloud |
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (0.995x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: String Encoding 📖Commits: PR vortex / vortex-file-compressed / ms (0.997x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / % (1.000x ➖, 0↑ 0↓)
|
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.001x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.911x ➖, 2↑ 2↓)
datafusion / parquet / ns (1.008x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.072x ➖, 1↑ 3↓)
duckdb / vortex-compact / ns (1.093x ➖, 0↑ 2↓)
duckdb / parquet / ns (0.977x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: TPC-H SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.005x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.998x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.980x ➖, 1↑ 0↓)
duckdb / vortex-compact / ns (0.988x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.996x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Clickbench Sorted on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.995x ➖, 1↑ 0↓)
datafusion / vortex-compact / ns (1.003x ➖, 0↑ 1↓)
datafusion / parquet / ns (1.011x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.996x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.014x ➖, 0↑ 1↓)
duckdb / parquet / ns (0.997x ➖, 2↑ 1↓)
File Size Changes (200 files changed, -0.0% overall, 99↑ 101↓)
Totals:
|
Benchmarks: FineWeb S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.797x ➖, 2↑ 0↓)
datafusion / vortex-compact / ns (0.824x ➖, 2↑ 0↓)
datafusion / parquet / ns (0.731x ➖, 5↑ 0↓)
duckdb / vortex-file-compressed / ns (0.859x ➖, 2↑ 0↓)
duckdb / vortex-compact / ns (0.912x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.730x ➖, 2↑ 0↓)
|
Benchmarks: Appian on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-compact / ns (1.000x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.003x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.010x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.023x ➖, 0↑ 1↓)
No file size changes detected. |
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.996x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.001x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.001x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.977x ➖, 3↑ 0↓)
duckdb / vortex-compact / ns (0.995x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.992x ➖, 0↑ 1↓)
No file size changes detected. |
Benchmarks: TPC-DS SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.999x ➖, 0↑ 2↓)
datafusion / vortex-compact / ns (0.992x ➖, 1↑ 0↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.000x ➖, 2↑ 2↓)
duckdb / vortex-compact / ns (1.000x ➖, 4↑ 3↓)
duckdb / parquet / ns (1.001x ➖, 3↑ 4↓)
No file size changes detected. |
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.993x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.997x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.984x ➖, 1↑ 0↓)
duckdb / vortex-compact / ns (0.997x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.996x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Statistical and Population Genetics 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.009x ➖, 0↑ 1↓)
datafusion / vortex-compact / ns (0.998x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.994x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.991x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.999x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.001x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Compression 📖Commits: PR vortex / vortex-file-compressed / ns (0.981x ➖, 0↑ 1↓)
vortex / vortex-file-compressed / bytes (1.000x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / ratio (0.991x ➖, 0↑ 1↓)
vortex / parquet / ns (0.995x ➖, 0↑ 0↓)
vortex / parquet / bytes (1.000x ➖, 0↑ 0↓)
vortex / arrow-ipc / ns (0.951x ➖, 5↑ 0↓)
vortex / arrow-ipc / bytes (1.000x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.964x ➖, 5↑ 6↓)
datafusion / vortex-compact / ns (0.896x ➖, 5↑ 3↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 3↓)
duckdb / vortex-file-compressed / ns (0.879x ➖, 6↑ 2↓)
duckdb / vortex-compact / ns (0.880x ➖, 2↑ 1↓)
duckdb / parquet / ns (1.068x ➖, 0↑ 3↓)
|
Benchmarks: TPC-H SF=10 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-compact / ns (0.742x ➖, 8↑ 1↓)
datafusion / parquet / ns (0.663x ✅, 15↑ 0↓)
duckdb / vortex-compact / ns (0.907x ➖, 1↑ 1↓)
duckdb / parquet / ns (0.770x ➖, 4↑ 1↓)
|
Benchmarks: Random Access 📖Commits: PR How to read Verdict and Engines
vortex / arrow-ipc / ns (0.940x ➖, 1↑ 0↓)
random-access / vortex-file-compressed / ns (0.982x ➖, 1↑ 0↓)
random-access / parquet / ns (0.993x ➖, 0↑ 0↓)
random-access / lance / ns (0.988x ➖, 0↑ 0↓)
|
Benchmarks: Random Access (S3) 📖Commits: PR How to read Verdict and Engines
random-access / vortex-file-compressed / ns (0.934x ➖, 4↑ 4↓)
random-access / parquet / ns (1.049x ➖, 0↑ 1↓)
random-access / lance / ns (0.855x ➖, 3↑ 2↓)
|
803a0e9 to
bef21a8
Compare
VortexError carried one enum variant per dependency that could fail
(Arrow, FlatBuffers, ObjectStore, Jiff, Tokio, TryFromInt, Prost, Fmt)
plus two structural wrappers (Context, Shared). None were constructed by
name outside the `From` impls or matched on, and the only classification
callers could act on, the `vx_error_code` the C API exports, reported
OTHER for nearly everything: 1966 `vortex_err!` sites and all 1133
`vortex_ensure!` and `vortex_panic!` sites gave no kind at all. It was the
equivalent of a codebase that only ever raises bare `Exception`.
The error type
VortexError is now a struct holding a `VortexErrorKind`, a message, an
optional source and a backtrace, `Arc`-backed so it is cheaply `Clone`.
Errors from dependencies are classified by what they mean to Vortex and
kept as `Error::source`, where callers downcast. `Context` becomes a
message prefix that preserves kind, source and backtrace; `Shared` goes
away. `size_of::<VortexError>()` drops from 88 bytes to 56, and the `?`
conversion path no longer formats eagerly.
The kinds are the C API's existing codes with two additions and one
removal:
- NotFound, for a name lookup that found nothing: a missing field, an
unknown encoding id, an absent segment. Python raises KeyError here.
- Overflow, for a numeric value that will not fit its target, the largest
group that was hiding in Other. `From<TryFromIntError>` produces it, so
`?` on a `try_from` classifies itself.
- Compute is removed. Every other kind names what went wrong; Compute named
where it happened, which the enum's own docs say a kind must not do. Its
11 sites were all Overflow, MismatchedTypes or Serde.
NotFound and Overflow take C codes 10 and 11; Compute's slot 2 is retired
and not reused, so no existing code moves. cbindgen regenerated
cinclude/vortex.h and the hand-written C++ mirror in lang/cpp matches.
The macros
`vortex_err!`, `vortex_bail!`, `vortex_ensure!`, `vortex_ensure_eq!` and
`vortex_panic!` now require a leading `Kind:` before any message; the
untagged arm is replaced by a compile error naming the fix, with
compile_fail doctests keeping it that way. The three canned arms
(`MismatchedTypes: expected, actual` and the like) are gone: a literal
is an expr, so `vortex_bail!(MismatchedTypes: "Expected {:?}", x)`
matched the canned arm and rendered its braces verbatim. The 32 sites
that used them spell their message out. The macros no longer inject
`use std::backtrace::Backtrace;` into caller scope.
`vortex_ensure_eq!(a, b)` with no message still reports AssertionFailed,
showing both expressions and values. A check on the caller's input names
its kind alone, `vortex_ensure_eq!(a, b, InvalidArgument)`, and keeps that
output.
The sites
Every construction site now names its kind, each chosen by reading
the site. The rules that settle the ambiguous ones: a check on what the
caller passed in is InvalidArgument or MismatchedTypes, and a length
mismatch is never a type error; a check that a kernel, rewrite or executor
kept its own contract is AssertionFailed; a check made while decoding
metadata or a wire format is Serde; a child index nobody can satisfy is
OutOfBounds however it is phrased. Io covers operating-system and
device-driver calls, so the CUDA, nvcomp and CUB status failures are Io,
as Python's OSError would be. The sites left Other carry it explicitly,
so each reads as a decision.
`From<ArrowError>` sent every Arrow failure to Other; each variant now maps
to its kind (ArithmeticOverflow to Overflow, SchemaError to
MismatchedTypes, the file-format variants to Serde, and so on).
`From<jiff::Error>` maps a date that does not exist or a misused unit to
InvalidArgument. pco's Corruption and InsufficientData map to Serde.
44 sites wrapped an inner VortexError in a fresh `vortex_err!`, which
replaced its kind with a guess and nested its whole Display, backtrace
included, in the new message. A text scan cannot tell those from the same
shape around an io::Error, so a throwaway build probed every `map_err`
closure with a marker type whose must_use warning fires only for a
VortexError. The 29 that wrapped edition-registration calls lose the
`map_err`; the rest use `with_context`, or the `vortex_panic!(err, "...")`
form, which keep the kind. STYLE.md now says to do that.
Python
The Python bindings turned every VortexError into RuntimeError, so none of
this reached Python. Each kind now raises its analogue:
OutOfBounds IndexError
NotFound KeyError
Overflow OverflowError
InvalidArgument ValueError
Serde ValueError
MismatchedTypes TypeError
NotImplemented NotImplementedError
AssertionFailed AssertionError
Io OSError
Other RuntimeError
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
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
bef21a8 to
74a7141
Compare
Instead of error being an enum we convert an error to be a struct with message and error kind. I had followed the structure of Python errors which have couple error categories https://docs.python.org/3/builtins/exceptions.html#exception-hierarchy majority of which don't apply to this project