Skip to content

perf: avoid per-row String allocation in Spark bin and char#23881

Merged
neilconway merged 1 commit into
apache:mainfrom
andygrove:opt/spark-bin-char
Jul 25, 2026
Merged

perf: avoid per-row String allocation in Spark bin and char#23881
neilconway merged 1 commit into
apache:mainfrom
andygrove:opt/spark-bin-char

Conversation

@andygrove

@andygrove andygrove commented Jul 25, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A

Rationale for this change

Two Spark functions allocated a String per row purely to render a small,
bounded amount of text:

  • bin called format!("{value:b}") for every row.
  • char called ch.to_string() for every row — a heap allocation for a single
    character.

In both cases the output has a known upper bound (64 binary digits for an i64,
4 bytes for a UTF-8 character), so the rendering fits in a stack buffer and the
result can be appended straight to a pre-sized builder.

What changes are included in this PR?

math/bin.rs:

  • spark_bin now writes digits right-aligned into a caller-supplied [u8; 64]
    and returns a &str borrowed from it, instead of returning an owned String.
  • The collect::<StringArray>() becomes an explicit loop over a
    StringBuilder::with_capacity, sized at 8 digits per row.
  • Negative values still render as their two's-complement bit pattern, matching
    {:b}. The digit loop is a loop, not a while, so zero renders as "0"
    rather than the empty string.

string/char.rs:

  • ch.to_string() becomes ch.encode_utf8(&mut encoded) against a [u8; 4]
    hoisted out of the loop.

Output is unchanged in both cases.

Are these changes tested?

Existing coverage pins the behaviour. spark/math/bin.slt asserts concrete
output for the cases the rewrite had to get right: zero, negative values,
i64::MIN (-9223372036854775808), i64::MAX, and -2147483648 /
-32768 widened from narrower integer types. spark/string/char.slt covers
the negative-input empty string, the null path, and characters on both sides of
the ASCII boundary (char(256) and above wrap via % 256).

All 119 spark/math and spark/string sqllogictest files pass, along with the
258 datafusion-spark unit tests.

The bin benchmark used below, datafusion/spark/benches/bin.rs, is added
separately in #23882 so the baseline can be measured on main before this
change lands. It covers 1024 and 8192 rows with 20% nulls over two value
distributions: small values that render to a handful of digits, and full-range
values that render to the maximum 64. char already had
datafusion/spark/benches/char.rs on main, so no benchmark change is needed
for it.

Benchmarks

Criterion, apache/main @ f1ab86dad as baseline. Median of the reported
change interval.

Benchmark 1024 8192
bin/small −75.7% −73.7%
bin/wide −47.8% −49.8%

char (1024 rows): −76.1%.

Are there any user-facing changes?

No. Both functions produce byte-identical output; this is purely an allocation
change.

bin formatted every row with format!("{value:b}"), and char built a String
for a single character with ch.to_string(), before collecting into a
StringArray.

Both render into a stack buffer instead: bin writes its digits right-aligned
in a [u8; 64] and char uses char::encode_utf8 into a [u8; 4]. Values are
appended to a pre-sized StringBuilder rather than collected.

bin -74% on small values and -50% on full-width ones, char -76%. The bin
benchmark is added in apache#23882.
@andygrove
andygrove force-pushed the opt/spark-bin-char branch from 033cd58 to 8c59488 Compare July 25, 2026 14:08
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.65%. Comparing base (f1ab86d) to head (8c59488).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23881      +/-   ##
==========================================
- Coverage   80.65%   80.65%   -0.01%     
==========================================
  Files        1091     1091              
  Lines      371031   371043      +12     
  Branches   371031   371043      +12     
==========================================
- Hits       299256   299254       -2     
- Misses      53935    53941       +6     
- Partials    17840    17848       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

lgtm!

@neilconway
neilconway added this pull request to the merge queue Jul 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 25, 2026
@neilconway
neilconway added this pull request to the merge queue Jul 25, 2026
Merged via the queue into apache:main with commit 36c417b Jul 25, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants