Skip to content

fix: sort ASCII-safe strings by runtime kind#868

Open
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:fix/sort-asciisafe-kind
Open

fix: sort ASCII-safe strings by runtime kind#868
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:fix/sort-asciisafe-kind

Conversation

@He-Pin
Copy link
Copy Markdown
Contributor

@He-Pin He-Pin commented May 23, 2026

Motivation:
Val.AsciiSafeStr is a Val.Str subclass used by renderer/string optimizations. std.sort was using exact runtime class equality to validate and dispatch sortable values, so arrays containing AsciiSafeStr could fail even though Jsonnet semantics treat those values as strings.

Key Design Decision:
Classify values by Jsonnet runtime kind rather than exact JVM/Native class. This preserves the existing specialized string/number/array sort paths while making subtype-based string optimizations semantically transparent to the standard library.

Modification:

  • Add small integer sort-kind classification in SetModule.
  • Replace exact getClass checks in default sorting and key-function sorting with sort-kind checks.
  • Add a regression in new_test_suite/set_sort_ascii_safe_strings.jsonnet for all-AsciiSafeStr, mixed Str/AsciiSafeStr, and key-function-produced AsciiSafeStr sorting.

Benchmark Results:
This is a correctness/enabler PR, not a claimed performance win. On the Scala Native 0.5.12 stacked profiling branch, kube-prometheus output stayed byte-identical; sort-kind-only A/B was neutral: forward 194.3 ± 14.6 ms clean vs 187.9 ± 12.3 ms candidate, reversed 170.7 ± 2.7 ms clean vs 172.0 ± 1.6 ms candidate.

Analysis:
The exact-class check was brittle because optimized runtime value subclasses must remain semantically indistinguishable from their base Jsonnet type. The replacement also avoids closure-based forall(_.getClass == keyType) checks in this path, but the important result is that future AsciiSafeStr propagation can be evaluated without breaking std.sort.

References:

  • Discovered while evaluating broader short-string AsciiSafeStr propagation for the Scala Native 0.5.12 performance work.

Result:

  • ./mill --no-server --ticker false --color false __.reformat passed.
  • ./mill --no-server --ticker false --color false -j 1 __.test passed 444/444.

Motivation:
AsciiSafeStr is a Val.Str subclass used by renderer optimizations, but std.sort compared exact runtime classes. Sorting arrays containing ASCII-safe strings could fail even though Jsonnet semantics treat them as strings.

Modification:
Classify sortable values by Jsonnet runtime kind instead of exact JVM/Native class, preserving existing fast paths for strings, numbers, arrays, booleans, and objects. Add a regression covering all-AsciiSafeStr, mixed Str/AsciiSafeStr, and key-function-produced AsciiSafeStr sorting.

Result:
std.sort now treats AsciiSafeStr values as strings without changing Jsonnet semantics. Full cross-platform validation will be run before opening the draft PR.
@He-Pin He-Pin marked this pull request as ready for review May 23, 2026 13:33
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