Skip to content

perf: optimize substring (up to 10x faster)#4903

Open
andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:auto-opt/substring-datafusion-comet-20260713-075248
Open

perf: optimize substring (up to 10x faster)#4903
andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:auto-opt/substring-datafusion-comet-20260713-075248

Conversation

@andygrove

@andygrove andygrove commented Jul 13, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A

Rationale for this change

Optimize existing expression.

What changes are included in this PR?

Replaced Arrow's substring_by_char with an implementation that skips the per-element chars().count() (unused for non-negative starts) and adds a bulk ASCII fast path that slices windows by byte offset without decoding UTF-8.

How are these changes tested?

Existing tests + new unit tests.

Benchmark (criterion):

  • substring_ long ascii tail: 90.315% faster (base 1049866ns -> cand 101681ns)
  • substring_ long ascii prefix: 45.268% faster (base 161701ns -> cand 88502ns)
  • substring_ short ascii: 69.169% faster (base 89039ns -> cand 27451ns)
  • substring_ long utf8 tail: 34.768% faster (base 3226354ns -> cand 2104613ns)
  • substring_ long utf8 prefix: 63.96% faster (base 365291ns -> cand 131650ns)

Full criterion output:

substring: short ascii  time:   [27.371 µs 27.413 µs 27.457 µs]
                        change: [−69.325% −69.169% −69.016%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  6 (6.00%) low mild
  2 (2.00%) high mild
  2 (2.00%) high severe

substring: long ascii prefix
                        time:   [88.107 µs 88.511 µs 88.914 µs]
                        change: [−45.520% −45.268% −45.006%] (p = 0.00 < 0.05)
                        Performance has improved.

substring: long ascii tail
                        time:   [101.42 µs 101.97 µs 102.50 µs]
                        change: [−90.371% −90.315% −90.260%] (p = 0.00 < 0.05)
                        Performance has improved.

substring: long utf8 prefix
                        time:   [131.49 µs 131.61 µs 131.77 µs]
                        change: [−64.048% −63.960% −63.868%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  3 (3.00%) high mild
  6 (6.00%) high severe

substring: long utf8 tail
                        time:   [2.1015 ms 2.1046 ms 2.1087 ms]
                        change: [−34.887% −34.768% −34.620%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  9 (9.00%) high severe

@andygrove andygrove changed the title perf: optimize substring in datafusion-comet-spark-expr perf: optimize substring (up to 10x faster) Jul 13, 2026
@andygrove andygrove marked this pull request as ready for review July 13, 2026 14:33
@andygrove

Copy link
Copy Markdown
Member Author

I created an upstream PR in arrow-rs as well: apache/arrow-rs#10334

@mbutrovich mbutrovich 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.

Thanks @andygrove! Thanks for also upstreaming!


/// Byte bounds of the `length`-codepoint window starting at codepoint `start` of an
/// arbitrary UTF-8 string, where a negative `start` counts back from the end.
#[inline]

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.

This feels more complex than what I'm used to seeing out of an #[inline]. However, it only has one call site so I'm not terrified of code size explosion.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants