Skip to content

perf: use ASCII bitsets for strip chars#789

Draft
He-Pin wants to merge 1 commit intodatabricks:masterfrom
He-Pin:perf/strip-ascii-bitset
Draft

perf: use ASCII bitsets for strip chars#789
He-Pin wants to merge 1 commit intodatabricks:masterfrom
He-Pin:perf/strip-ascii-bitset

Conversation

@He-Pin
Copy link
Copy Markdown
Contributor

@He-Pin He-Pin commented Apr 26, 2026

Motivation:

std.stripChars, std.lstripChars, and std.rstripChars now already avoid boxed Set[Int] for single/BMP delimiter sets on master. This PR keeps that Unicode-aware path, and narrows the ASCII-only multi-character delimiter case further by replacing the per-call java.util.BitSet allocation with primitive masks.

Key Design:

  • ASCII-only delimiter sets use primitive masks before the BMP fallback.
  • JVM and Scala.js use four Int masks; Scala Native uses two Long masks.
  • Non-ASCII BMP delimiter sets keep the upstream BitSet fast path.
  • Surrogate-containing delimiter sets still fall back to codepoint iteration.
  • chars is forced before str, preserving existing error evaluation order.

Benchmark Results:

Rebased onto upstream/master at 8b67cb1e (Add workaround to make CI more reliable (#824)).

JMH full regression command:

./mill -i bench.runRegressions

Full JMH ran all 45 regression cases on master and this PR. There is no stable standout runtime delta after rebase. The strip-char cases are within normal local JMH noise:

  • lstripChars.jsonnet: 0.111 -> 0.112 ms/op
  • rstripChars.jsonnet: 0.113 -> 0.114 ms/op
  • stripChars.jsonnet: 0.111 -> 0.110 ms/op

Native hyperfine command shape:

hyperfine --shell=none --warmup 5 --runs 50 --time-unit millisecond \
  -L case lstripChars.jsonnet,rstripChars.jsonnet,stripChars.jsonnet \
  '<master-native> bench/resources/go_suite/{case}' \
  '<this-pr-native> bench/resources/go_suite/{case}' \
  '<jrsonnet> bench/resources/go_suite/{case}'

Native CLI also has no standout delta vs current master:

  • lstripChars.jsonnet: 3.81 -> 3.80 ms; jrsonnet 2.00 ms
  • rstripChars.jsonnet: 3.74 -> 3.81 ms; jrsonnet 2.02 ms
  • stripChars.jsonnet: 3.82 -> 3.87 ms; jrsonnet 2.13 ms

Analysis:

The earlier positive JMH signal disappeared after rebasing because current master already has the BMP BitSet strip fast path. This PR should be treated as a small GC-friendly allocation cleanup for ASCII delimiter sets, not as a visible improvement on the go_suite strip benchmarks or the jrsonnet gap.

Validation:

  • ./mill -i __.checkFormat: pass
  • git diff --check: pass
  • ./mill -i 'sjsonnet.jvm[3.3.7].test': pass
  • ./mill -i 'sjsonnet.js[3.3.7].test': pass
  • ./mill -i 'sjsonnet.native[3.3.7].test': pass
  • ./mill -i bench.runRegressions: pass on master and this PR
  • Native hyperfine --shell=none --warmup 5 --runs 50: pass

@He-Pin He-Pin marked this pull request as draft April 26, 2026 17:53
Port the useful part of He-Pin/sjsonnet jit commit dd90b11 onto current master. The new path avoids Set[Int] allocation for ASCII-only stripChars, lstripChars, and rstripChars calls, preserves the existing codepoint-aware fallback for non-ASCII strip sets, and uses platform-specific masks selected from local measurements.

Upstream-source: dd90b11
@He-Pin He-Pin force-pushed the perf/strip-ascii-bitset branch from 9004941 to 7f64a1d Compare May 7, 2026 19:49
@He-Pin
Copy link
Copy Markdown
Contributor Author

He-Pin commented May 8, 2026

Closing as low priority after rebase: current master already has the main strip-char fast path, and this PR shows no stable standout delta on the go_suite strip docs cases or Native hyperfine.

@He-Pin He-Pin closed this May 8, 2026
@He-Pin He-Pin reopened this May 8, 2026
@He-Pin
Copy link
Copy Markdown
Contributor Author

He-Pin commented May 8, 2026

Reopened. This is not negative; keep it as draft as a small GC-friendly strip delimiter cleanup. Current docs strip benchmarks do not show standout gains, so it should not be moved to ready yet.

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.

1 participant