Skip to content

Read VarBin UTF-8 RowFn inputs through their offsets - #10340

Merged
connortsui20 merged 1 commit into
developfrom
ct/row-fn-utf8-varbin-offsets
Oct 7, 2026
Merged

connortsui20 merged 1 commit into
developfrom
ct/row-fn-utf8-varbin-offsets

Conversation

@connortsui20

@connortsui20 connortsui20 commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Summary

Replaces #10262 and #10263, whose offset elements needed a layout choice before RowFn::dispatch, which only sees dtypes. Here Utf8Column picks its storage per batch, so every existing RowFn skips building a 16-byte view per row for VarBin input.

Changes

Offsets that do not fit in u32, or that delimit invalid UTF-8 at any row (null rows included), fall back to string views. The row accessors are now #[inline] because they are not generic, so row loops in other crates made one call per row, which is why VarBinView input also got faster.

On the new row_fn_utf8_input bench, a starts_with RowFn over 16,384 rows goes from 227.5 µs to 34.1 µs on VarBin with 8-byte values and from 79.7 µs to 65.3 µs on VarBinView with 8-byte values.

Benchmark methodology and full results

Baseline is develop at 659c7f7. Candidate is this commit before its rebase onto d9ad4cf (a51871e). The two binaries ran alternately 7 times on a 4-core cloud VM with the bench profile. Each figure is the median of the 7 per-run divan medians. A later rerun of the rebased commit with only doc and comment changes matched these numbers.

Input develop This PR
VarBin, 8-byte values 227.5 µs 34.1 µs
VarBin, 64-byte values 252.9 µs 47.9 µs
VarBinView, 8-byte values 79.7 µs 65.3 µs
VarBinView, 64-byte values 120.4 µs 113.1 µs

API Changes

Utf8Column now yields &str, and Utf8View is removed. Callers that only dereference the value compile unchanged, but x == value.as_ref() becomes ambiguous because str implements AsRef for several targets, so compare against value directly. A null row still yields an unspecified string, which offset storage now takes from the stored bytes instead of an empty string.

🤖 Generated with Claude Code

https://claude.ai/code/session_018FefTpTuDky8bGPr5R9Loq

@codspeed

codspeed Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by 12.73%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 2110 untouched benchmarks
🆕 12 new benchmarks
⏩ 518 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ WallTime bitpack_blocked_compress_avx2 7.6 µs 6.8 µs +12.73%
🆕 WallTime starts_with_avx2[64, VarBin] N/A 57.3 µs N/A
🆕 WallTime starts_with_avx2[64, VarBinView] N/A 124 µs N/A
🆕 WallTime starts_with_avx2[8, VarBin] N/A 41.5 µs N/A
🆕 WallTime starts_with_avx2[8, VarBinView] N/A 75.3 µs N/A
🆕 WallTime starts_with_avx512[64, VarBin] N/A 56.7 µs N/A
🆕 WallTime starts_with_avx512[64, VarBinView] N/A 124.6 µs N/A
🆕 WallTime starts_with_avx512[8, VarBin] N/A 41 µs N/A
🆕 WallTime starts_with_avx512[8, VarBinView] N/A 70.9 µs N/A
🆕 WallTime starts_with_neon[64, VarBin] N/A 65.6 µs N/A
🆕 WallTime starts_with_neon[64, VarBinView] N/A 114.9 µs N/A
🆕 WallTime starts_with_neon[8, VarBin] N/A 44.2 µs N/A
🆕 WallTime starts_with_neon[8, VarBinView] N/A 76.9 µs N/A

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ct/row-fn-utf8-varbin-offsets (040d48c) with develop (9eb20ba)2

Open in CodSpeed

Footnotes

  1. 518 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. ↩

  2. No successful run was found on develop (46d518b) during the generation of this report, so 9eb20ba was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

connortsui20 commented Oct 6, 2026 •

Copy link
Copy Markdown
Member Author

Edit: CodSpeed no longer flags random_i128[0.01] on 040d48c, the rebase onto 9eb20ba, and reports no regressions.

The CodSpeed check was red only because of random_i128[0.01] (filter_fixed_width, Simulation, reported 11.68% slower). That benchmark filters i128 primitives and runs none of the code this PR changes.

Under callgrind, the random_i128 benchmarks run 87,836,916 instructions on develop (659c7f7) and 87,837,204 on this PR: 288 more instructions in total (0.0003%). The reported change therefore comes from code placement in CodSpeed's cache simulation, not from extra work. The same benchmark also moves on unrelated PRs (42.9 µs to 9.7 µs on #10329).


Generated by Claude Code

@connortsui20
connortsui20 force-pushed the ct/row-fn-utf8-varbin-offsets branch from a51871e to 967721d Compare October 7, 2026 09:45
@connortsui20
connortsui20 marked this pull request as ready for review October 7, 2026 09:48
@connortsui20
connortsui20 force-pushed the ct/row-fn-utf8-varbin-offsets branch from 967721d to 90eae7e Compare October 7, 2026 09:55

connortsui20 commented Oct 7, 2026 •

Copy link
Copy Markdown
Member Author

Edit: #10352 merged before CI ran on 040d48c. PR CI tests the merge with develop, so Python (lint) passes there without another branch update.

Python (lint) fails on 90eae7e because of ty 0.0.85, released on October 6 at 22:05 UTC. CI runs uvx ty check unpinned, so it picked up two new diagnostics in files this PR does not touch: a @contextmanager in vortex-python/python/vortex/dataset.py and a redundant cast in vortex-python/test/test_expr.py.

The last develop run passed before that release, so develop and every other PR now fail the same way, and a rerun would fail again. The fix is in #10352.


Generated by Claude Code

`Utf8Column` executed every input to string views, so a `VarBin` input built a
16-byte view per row before any callback ran. It now borrows the `VarBin`
offsets and bytes directly when the offsets fit in `u32` and delimit valid
UTF-8, null rows included. Every other input still executes to views.

Row callbacks now receive `&str`, which both storages produce, so `Utf8View`
is removed. The row accessors carry `#[inline]`. They are not generic, so
without it a row loop instantiated in another crate made a function call per
row.

`VarBinData::validate_utf8` already checked whether offsets delimit valid
UTF-8. That check moves into `offsets_tile_utf8` so both paths share it.

Signed-off-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018FefTpTuDky8bGPr5R9Loq
@connortsui20
connortsui20 force-pushed the ct/row-fn-utf8-varbin-offsets branch from 90eae7e to 040d48c Compare October 7, 2026 10:22
@connortsui20
connortsui20 merged commit 9324b04 into develop Oct 7, 2026
90 checks passed
@connortsui20
connortsui20 deleted the ct/row-fn-utf8-varbin-offsets branch October 7, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/performance A performance improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants