Skip to content

Read RowFn UTF-8 inputs from native offsets - #10262

Closed
connortsui20 wants to merge 1 commit into
developfrom
ct/row-fn-utf8-offsets
Closed

connortsui20 wants to merge 1 commit into
developfrom
ct/row-fn-utf8-offsets

Conversation

@connortsui20

Copy link
Copy Markdown
Member

Summary

String RowFns currently construct 16-byte string views from VarBin inputs before callbacks can read them. Utf8OffsetColumn makes direct access to validated native u32 offsets available through the existing input-element API, while retaining payload ownership and strict null propagation.

Changes

Selection is explicit because RowFn::dispatch receives logical dtypes. Other nonempty layouts are rejected, and Utf8Column remains the general decoder. Batch constants use an opaque validated value so safe callers cannot pass arbitrary bytes to unchecked string access.

This remains a draft pending a consumer integration and measurements of the exact API port. Decode reports fallibility because it can reject a physical layout, so partially valid batches use the existing valid-only policy. The research prototype declared infallible decode, and its percentages do not qualify this port. Existing native LIKE specializations also remain stronger baselines in several cases.

Research evidence, before the API port

At develop 9a08b82dc683cd3fff8dcde834c382141f425f48, a generic prefix RowFn on 65,536 dense 64-byte VarBin strings measured 919,427 ns/batch through Utf8Column and 418,615 ns through the private offset prototype. The median paired ratio was 0.450, with five process-pair ratios from 0.444 to 0.465. This removes 1,048,576 bytes of view metadata for one input, while both paths share the payload and validate readable UTF-8.

Measurements used c7i.4xlarge, Rust 1.98.0, mimalloc, AVX2, the benchmark profile with 16 codegen units and LTO disabled, CPU affinity with the SMT sibling offline, and five alternating process pairs with 30 samples per process. This comparison measures the research binding, not the renamed library port or a query. Native LIKE must remain available, and there is no production CPU-share evidence supporting a query-gain claim.

@codspeed

codspeed Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by 11.98%

⚠️ 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
✅ 2100 untouched benchmarks
⏩ 518 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ WallTime scalar_subtract_neon 13.5 µs 12 µs +11.98%

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-offsets (41ae7ce) with develop (c6e51ba)

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

@connortsui20
connortsui20 added this pull request to stack #10264 October 3, 2026 02:09
@connortsui20
connortsui20 force-pushed the ct/row-fn-utf8-offsets branch from 9551ecc to 9e95391 Compare October 6, 2026 11:38
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
connortsui20 force-pushed the ct/row-fn-utf8-offsets branch from 9e95391 to 41ae7ce Compare October 6, 2026 11:50

connortsui20 commented Oct 6, 2026 •

Copy link
Copy Markdown
Member Author

Closing in favor of a simpler change: #10340.

Instead of adding an opt-in Utf8OffsetColumn, Utf8Column itself now reads a VarBin input through its offsets when every offset fits in u32 and every row (null rows included) is valid UTF-8. Every other input still executes to string views. So every existing RowFn gets the fast path without choosing a layout before dtype-only dispatch, and no new public element type is needed. It also covers i32 offsets (Arrow Utf8 imports) without a copy.

Measured with the new row_fn_utf8_input bench (a starts_with RowFn, 16,384 rows, median of 7 alternating runs):

Input develop New
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

The VarBinView gain comes from marking the row accessors #[inline]. They are not generic, so a row loop instantiated in another crate called them once per row.


Generated by Claude Code

@connortsui20
connortsui20 deleted the ct/row-fn-utf8-offsets branch October 6, 2026 15:19
connortsui20 added a commit that referenced this pull request Oct 7, 2026
## 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.

<details>
<summary>Benchmark methodology and full results</summary>

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 |

</details>

## 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.com/claude-code)

https://claude.ai/code/session_018FefTpTuDky8bGPr5R9Loq

Signed-off-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant