Skip to content

perf: speed up substring_by_char with an ASCII fast path and single-pass bounds#10334

Open
andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:optimize-substring-by-char
Open

perf: speed up substring_by_char with an ASCII fast path and single-pass bounds#10334
andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:optimize-substring-by-char

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A — performance improvement to an existing kernel.

Rationale for this change

substring_by_char calls val.chars().count() on every value, which is a full UTF-8 decode of the entire string. The result is only used when start < 0, so for the common non-negative start case the work is thrown away. When start < 0, the string is then walked a second time from the front to convert the char index back into a byte offset.

It also decodes UTF-8 unconditionally, even when the array is entirely ASCII, where a char index is a byte index and the bounds can be computed arithmetically. The doc comment currently tells users to reach for substring themselves in that case; the kernel can just detect it.

This is a port of the approach taken in apache/datafusion-comet#4903, which replaced this kernel with a local copy for exactly these reasons.

What changes are included in this PR?

  • substring_by_char picks a bounds function once per array: ascii_bounds (integer arithmetic on byte offsets) when is_ascii() holds, utf8_bounds otherwise. substring_by_char_impl does the shared buffer building.
  • The unconditional chars().count() is gone. Negative starts use char_indices().nth_back(), which decodes only as far back as needed rather than scanning the whole value twice.
  • utf8_bounds short-circuits the end scan when the requested char length is at least the remaining byte length, since a char is never smaller than one byte.
  • The value buffer capacity is bounded by an upper estimate of the output element size, so a short substring of long strings no longer allocates the full input size.
  • Semantics are unchanged, including the clamping of out-of-range starts in both directions.

The output is still built with the checked GenericStringArray::new. Skipping the redundant UTF-8 validation with new_unchecked is possible (every slice is on a char boundary of an already-valid string) but is left out of this PR.

Benchmark, 65,536 rows x 1,000-char strings:

Benchmark Before After Change
substring utf8 by char (existing) 33.97 ms 4.84 ms -85.7%
ascii, prefix (start=0, length=10) 2.61 ms 2.34 ms -10.1%
ascii, tail (start=-10) 31.76 ms 2.41 ms -92.4%
non-ascii, prefix 3.95 ms 1.34 ms -66.1%
non-ascii, tail 65.62 ms 1.06 ms -98.4%

The ASCII prefix case gains the least because it is dominated by the copy and the output validation, but it still comes out ahead of the is_ascii() scan it now pays for.

Are these changes tested?

Yes. The existing substring_by_char tests all mix ASCII and non-ASCII values in a single array, so they only ever exercised the UTF-8 path. Added an ASCII-only test matrix (ascii_string_by_char / ascii_large_string_by_char) covering identity, positive and negative starts, out-of-range starts in both directions, zero length, and a u64::MAX length to pin the saturating-add clamp.

Added four benchmarks to substring_kernels.rs (ASCII and non-ASCII, prefix and tail) plus a non-ASCII array generator; the existing bench name is unchanged so its history stays comparable.

Are there any user-facing changes?

No API change. substring_by_char is faster, and its # Performance doc note is updated to mention the ASCII fast path.

…ass bounds

Avoid the unconditional chars().count() on every value, use nth_back for
negative starts, and compute byte bounds arithmetically when the array is
all ASCII.
@Rich-T-kid

Rich-T-kid commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

I can take a look at this tomorrow morning.
I was curious looking at the PR diff, would it make sense to make the benchmarks a seperate PR so we could run them on @adriangbot?

@Jefffrey Jefffrey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

running the benchmarks locally, great results

substring utf8 (start = 0, length = None)
                        time:   [2.5852 ms 2.5907 ms 2.5969 ms]
                        change: [+0.2196% +0.5704% +0.9392%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 14 outliers among 100 measurements (14.00%)
  11 (11.00%) low mild
  1 (1.00%) high mild
  2 (2.00%) high severe

substring utf8 (start = 1, length = str_len - 1)
                        time:   [3.5970 ms 3.6051 ms 3.6148 ms]
                        change: [+0.2273% +0.6151% +1.0087%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 8 outliers among 100 measurements (8.00%)
  1 (1.00%) high mild
  7 (7.00%) high severe

substring utf8 by char  time:   [4.6956 ms 4.7021 ms 4.7096 ms]
+                       change: [−87.045% −87.012% −86.979%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 26 outliers among 100 measurements (26.00%)
  7 (7.00%) low severe
  3 (3.00%) low mild
  6 (6.00%) high mild
  10 (10.00%) high severe

substring by char (ascii, prefix)
                        time:   [2.2969 ms 2.3041 ms 2.3126 ms]
-                       change: [+7.0461% +7.4037% +7.7906%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 20 outliers among 100 measurements (20.00%)
  1 (1.00%) low mild
  4 (4.00%) high mild
  15 (15.00%) high severe

substring by char (ascii, tail)
                        time:   [2.3013 ms 2.3083 ms 2.3167 ms]
+                       change: [−93.238% −93.214% −93.188%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  2 (2.00%) high mild
  6 (6.00%) high severe

Benchmarking substring by char (non-ascii, prefix): Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 5.6s, enable flat sampling, or reduce sample count to 60.
substring by char (non-ascii, prefix)
                        time:   [1.0798 ms 1.0989 ms 1.1202 ms]
+                       change: [−63.584% −63.142% −62.655%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  9 (9.00%) high mild
  1 (1.00%) high severe

substring by char (non-ascii, tail)
                        time:   [812.63 µs 823.64 µs 837.11 µs]
+                       change: [−98.695% −98.673% −98.649%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 19 outliers among 100 measurements (19.00%)
  18 (18.00%) high mild
  1 (1.00%) high severe

Benchmarking substring fixed size binary array: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.9s, enable flat sampling, or reduce sample count to 60.
substring fixed size binary array
                        time:   [1.3592 ms 1.3604 ms 1.3617 ms]
                        change: [−0.5443% −0.1591% +0.2104%] (p = 0.43 > 0.05)
                        No change in performance detected.
Found 13 outliers among 100 measurements (13.00%)
  3 (3.00%) high mild
  10 (10.00%) high severe
  • that regression is probably just noise on my machine 🤔

@alamb

alamb commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

run benchmarks substring

@adriangbot

Copy link
Copy Markdown

🤖 Arrow criterion benchmark running (GKE) | trigger
Instance: c4a-highmem-16 (12 vCPU / 65 GiB) | Linux bench-c4984704049-1088-84dx9 6.12.85+ #1 SMP Mon May 11 08:17:35 UTC 2026 aarch64 GNU/Linux

CPU Details (lscpu)
Architecture:                            aarch64
CPU op-mode(s):                          64-bit
Byte Order:                              Little Endian
CPU(s):                                  16
On-line CPU(s) list:                     0-15
Vendor ID:                               ARM
Model name:                              Neoverse-V2
Model:                                   1
Thread(s) per core:                      1
Core(s) per cluster:                     16
Socket(s):                               -
Cluster(s):                              1
Stepping:                                r0p1
BogoMIPS:                                2000.00
Flags:                                   fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache:                               1 MiB (16 instances)
L1i cache:                               1 MiB (16 instances)
L2 cache:                                32 MiB (16 instances)
L3 cache:                                80 MiB (1 instance)
NUMA node(s):                            1
NUMA node0 CPU(s):                       0-15
Vulnerability Gather data sampling:      Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit:             Not affected
Vulnerability L1tf:                      Not affected
Vulnerability Mds:                       Not affected
Vulnerability Meltdown:                  Not affected
Vulnerability Mmio stale data:           Not affected
Vulnerability Reg file data sampling:    Not affected
Vulnerability Retbleed:                  Not affected
Vulnerability Spec rstack overflow:      Not affected
Vulnerability Spec store bypass:         Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1:                Mitigation; __user pointer sanitization
Vulnerability Spectre v2:                Mitigation; CSV2, BHB
Vulnerability Srbds:                     Not affected
Vulnerability Tsa:                       Not affected
Vulnerability Tsx async abort:           Not affected
Vulnerability Vmscape:                   Not affected

Comparing optimize-substring-by-char (c708cd5) to 543b806 (merge-base) diff
BENCH_NAME=substring
BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench substring
BENCH_FILTER=
Results will be posted here when complete


File an issue against this benchmark runner

@adriangbot

Copy link
Copy Markdown

Benchmark for this request failed.

Last 20 lines of output:

Click to expand
    record_batch
    regexp_kernels
    row_format
    row_group_index_reader
    row_selection_cursor
    row_selector
    serde
    sort_kernel
    string_dictionary_builder
    string_run_builder
    string_run_iterator
    substring_kernels
    take_kernels
    union_array
    variant_builder
    variant_kernels
    variant_validation
    view_types
    writer_overhead
    zip_kernels

File an issue against this benchmark runner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants