perf: add ASCII-safe substr fast path#834
Open
He-Pin wants to merge 1 commit intodatabricks:masterfrom
Open
Conversation
340215d to
a64324e
Compare
a64324e to
c6cea80
Compare
Collaborator
|
rebase |
Motivation: std.substr on long ASCII strings repeatedly pays codepoint-offset scans even when parser-time analysis can prove the literal is printable ASCII and JSON-render safe. Modification: Mark long ASCII JSON-safe literals with the existing _asciiSafe flag using a single platform CharSWAR scan, propagate the flag through string concatenation, and let std.length/std.substr use direct UTF-16 length/substring only for proven-safe values. Add UnicodeHandlingTests coverage for long ASCII length/substr boundaries and concat propagation. Result: Focused JVM JMH improves go_suite/substr from 0.056 ms/op to 0.046-0.047 ms/op with split_resolve unchanged and realistic2 in the same noise range. Scala Native hyperfine is neutral against master on the same case. References: Extracted from ideas in databricks#776, especially commit a190a80 (ASCII fast paths and asciiSafe propagation), narrowed to avoid the broader join/parseInt changes.
c6cea80 to
35937b9
Compare
Contributor
Author
|
@stephenamar-db rebased |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Long printable ASCII strings can use UTF-16 length and substring indexes directly because every UTF-16 code unit is also one Jsonnet codepoint.
std.substrcurrently still pays codepoint offset checks for these proven-safe long literals.Key Design Decision:
Reuse the existing
_asciiSaferuntime marker, but only set it for long (>1024 chars) string literals that are printable ASCII and JSON-render safe. This keeps parser overhead off short-string-heavy workloads and avoids broad runtime ASCII scans. JVM uses byte-level SWAR for the long-string safety scan; Scala Native and Scala.js keep a scalar single-pass scan because the byte-buffer SWAR variant regressed Native due UTF-8 allocation cost.Modification:
CharSWAR.isAsciiJsonSafeon JVM, Native, and JS.Parser.makeStr._asciiSafethroughVal.Str.concatwhen both operands are safe.std.lengthandstd.substruse directString.length/substringonly for proven-safe strings.Benchmark Results:
JMH, lower is better (
./mill --no-server -j 1 bench.runRegressions ...):go_suite/substr.jsonnetjdk17_suite/split_resolve.jsonnetcpp_suite/realistic2.jsonnetScala Native hyperfine, lower is better (
--shell=none --warmup 5 --runs 20):go_suite/substr.jsonnetgo_suite/substr.jsonnetgo_suite/substr.jsonnetAnalysis:
The broad #776 ASCII/string fast-path ideas were narrowed aggressively. Runtime ASCII scans in
std.length/std.substrcaused guard regressions, and all-platform UTF-8 byte-SWAR regressed Scala Native. This version uses byte-SWAR only where it measured well (JVM), keeps Native/JS allocation-free, and only takes fast paths when the marker is already proven, preserving Jsonnet codepoint semantics for all non-ASCII strings.References:
Result:
Draft PR for review. JVM substr benchmark is positive; Native is neutral/slightly positive with no-shell hyperfine; full
./mill --no-server -j 1 __.reformat && ./mill --no-server -j 1 __.testpassed locally.