perf: speed up constant string joins#825
Merged
stephenamar-db merged 1 commit intodatabricks:masterfrom May 11, 2026
Merged
Conversation
7dd77cb to
9e7c7f1
Compare
Contributor
Author
|
Closing as superseded by #828, which carries the TOML and repeated join work with current docs-aligned benchmark data. |
Contributor
Author
|
Reopened and moved back to draft rather than closing. It is not negative, but it is currently superseded by #828, so it should not be ready for review unless we decide to keep it as a separate smaller join-only PR. |
Collaborator
|
rebase |
9e7c7f1 to
ca2d708
Compare
Collaborator
|
Let's do it one at a time here - please rebase this one and I'll merge |
8e8401c to
ca2d708
Compare
Motivation: std.join over std.repeat([string], n) currently walks every repeated element and repeatedly appends the same string/separator pattern. Modification: Teach repeated/constant arrays to expose a constant Eval and let std.join materialize repeated string results directly while preserving null skipping and lazy error behavior. Keep the existing array-join copy paths for non-string separators. Result: Adds directional golden coverage for repeated string, unicode string, null, array, and zero-length lazy-error join cases. References: PR: databricks#825
ca2d708 to
ce421ff
Compare
Contributor
Author
|
Rebased onto latest master and refreshed the local validation/benchmark data in the PR description. I also added directional golden coverage for repeated constant joins. |
Contributor
Author
|
@stephenamar-db I have rebased ,sorry for many conflicts |
stephenamar-db
approved these changes
May 11, 2026
stephenamar-db
pushed a commit
that referenced
this pull request
May 11, 2026
Motivation: `std.manifestTomlEx` spends avoidable time and allocation while rendering TOML: regex matching for bare keys, intermediate escaped strings, per-value renderer allocation, `Seq` path copies, `partition`, and repeated sorting during table traversal. Key Design Decision Keep this PR TOML-only. The earlier repeated-join work is now handled separately in #825, so this branch was intentionally reduced to a single focused TOML rendering commit after rebasing onto latest master. Modification: - Write TOML strings and keys directly to the existing `StringWriter`. - Replace bare-key regex matching with a small ASCII scanner. - Avoid string concatenations and unnecessary `StringWriter.flush()` calls while rendering arrays. - Reuse a `TomlRenderer` per table level and keep table paths in a mutable buffer. - Use sorted visible keys plus section flags instead of `partition` plus extra sorting. - Add directional golden coverage for TOML key escaping, non-section ordering, nested tables, and table arrays. Benchmark Results: JMH on JDK 21, `./mill -j 1 bench.runRegressions bench/resources/go_suite/manifestTomlEx.jsonnet`, lower `ms/op` is better; `ops/ms = 1 / ms_per_op`, higher is better. | Case | master ms/op | PR ms/op | master ops/ms | PR ops/ms | Delta | | --- | ---: | ---: | ---: | ---: | ---: | | `go_suite/manifestTomlEx.jsonnet` | 0.067 | 0.053 | 14.93 | 18.87 | +26.4% ops/ms | Scala Native hyperfine, lower is better, comparing locally built Scala Native binaries and source-built `jrsonnet 0.5.0-pre98`. | Case | master | PR | jrsonnet | Result | | --- | ---: | ---: | ---: | --- | | `go_suite/manifestTomlEx.jsonnet` | 5.3 ± 1.3 ms | 5.3 ± 1.3 ms | 4.0 ± 1.1 ms | startup/noise dominated; PR and master are neutral | | scaled TOML manifestation loop | 29.0 ± 1.6 ms | 12.1 ± 1.0 ms | 10.0 ± 0.9 ms | PR is ~58.3% faster than master; still ~1.21x jrsonnet | Analysis: The output remains compatible with current sjsonnet behavior: local master and this PR produced identical output on the benchmark inputs and the new directional TOML golden test. The scaled Native comparison includes jrsonnet as a performance reference, but jrsonnet's TOML formatting differs from sjsonnet on these inputs, so sjsonnet-vs-sjsonnet output equality is the semantic gate. References: - PR: #828 - Rebased head: `3efa453a5b6f48d6dfd0e1cd034922d6c8e2ff03` - Base: `0ae7b78a93c4e643f9bcfb6dd1d99d9fe7a522a9` - Split dependency: #825 now owns the repeated constant join optimization. Result: - `./mill --no-server -j 1 __.reformat` passed locally. - `./mill --no-server -j 1 __.test` passed locally. - PR comments were reviewed; there are no review threads. The maintainer rebase request is addressed by this force-pushed TOML-only branch.
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:
std.joinover repeated constant string arrays (for examplestd.join('', std.repeat(['e'], n))) currently walks every element and repeatedly appends the same separator/value pair. This PR keeps the optimization focused on that repeated/constant-array case while preserving Jsonnet laziness and null-skipping behavior.Key Design Decision
Expose a best-effort
constantEvalfrom array views and use it only whenstd.joincan prove every element is the same already-known string or null. If the element is lazy, non-string, or otherwise unsafe to inspect eagerly, the code falls back to the existing element-by-element join path.Modification:
std.joinfor constant/repeated string arrays with exact result sizing.Benchmark Results:
JMH on JDK 21,
./mill -j 1 bench.runRegressions ..., lowerms/opis better;ops/ms = 1 / ms_per_op, higher is better.cpp_suite/bench.09.jsonnetcpp_suite/large_string_join.jsonnetsjsonnet_suite/array_copy_views.jsonnetguard rerunScala Native hyperfine, lower is better, comparing locally built Scala Native binaries and source-built
jrsonnet 0.5.0-pre98.std.repeat(['abcd'], 50000))cpp_suite/large_string_join.jsonnetcpp_suite/bench.09.jsonnettiny CLI runAnalysis:
The semantic guard is intentionally conservative: only already-constant string/null elements take the new direct materialization path. Lazy errors are not forced early, as covered by
std.join('-', std.repeat([error 'unused'], 0)). Non-constant arrays and array-separator joins retain the existing master behavior.References:
ce421ff5fa002eaae136d5241b08d34685ba04d20ae7b78a93c4e643f9bcfb6dd1d99d9fe7a522a9Result:
./mill -j 1 __.reformat && ./mill -j 1 __.testpassed locally.