perf(kotlin): resolve operands without compiling a regex - #55
Merged
Conversation
`KotlinContext.asValue` built four `Regex` instances inline on the
operand path: two in the `when` that decides string literal vs field
path, two more in `unwrap`. `process` calls `asValue` on both operands,
so a single binary expression compiled up to four patterns on every
evaluation. `trim()` also ran twice in the same `when`.
The patterns collapse to one hoisted val, the string case is entered
once with a single `trim()`, and `unwrap` drops regex entirely.
```kotlin
private fun Any?.asValue(): Any? {
val result = when {
this !is String -> this
this == "null" -> null
else -> {
val trimmed = this.trim()
if (QUOTED.matches(trimmed)) trimmed.unwrap() else trimmed.rawValue()
}
}
...
}
private fun String.unwrap() = this.trim()
.removePrefix("\"")
.removeSuffix("\"")
```
`QUOTED` is unanchored. `Regex.matches` is a full-input match, so the
`^` and `$` in the original `Regex("^\".*\"$")` were redundant and both
`when` branches tested the same predicate. `.` still does not match a
newline, so a multiline quoted literal falls through to `rawValue` as
before.
`unwrap` uses `removePrefix`/`removeSuffix` and not
`removeSurrounding("\"")`. Those are not equivalent: `unwrap` strips a
leading and a trailing quote independently, `removeSurrounding` strips
only when both are present. `rawValue` calls `unwrap` on strings that
did not match `QUOTED`, so a key such as `"abc` resolves to `abc`
today and would resolve to `"abc` under `removeSurrounding`, changing
the map lookup and the throw/no-throw outcome for `OnFailure`.
No test changes.
The Performance section was the longest part of the README and is reference material, not something a reader needs while picking an engine. It moves to BENCHMARKS.md with the run-to-run spread of each engine recorded alongside the headline figures, since two of the four configurations vary by more than 30% between runs. The README keeps the engine comparison table and a link.
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
1 participant
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.
KotlinContext.asValuecompiled up to fourRegexpatterns per binary expression, once per evaluation, and trimmed the same string twice. Operand parsing is on the hot path of the engine people pick for throughput.Patterns hoisted to one
QUOTEDval, onetrim(), andunwrapmoved off regex toremovePrefix/removeSuffix.Results
./gradlew :kotlin-evaluator:bench -PbenchIterations=2000, 147-expression suite, three runs per variant in one session at full power. The middle row keepsunwrapon regex with its patterns hoisted, to separate the two effects:unwrapimplementationRegex(before)RegexvalsremovePrefix/removeSuffix(after)26% end to end, no overlap between the bands. Hoisting is ~11%, dropping regex a further ~13%. A compiled pattern is still slower:
replace(Regex, String)allocates aMatcherand walks the input every call, whileremovePrefixis astartsWithplus at most onesubstringand allocates nothing when the quote is absent, which is the common case since field paths carry no quotes. The tail shows it clearest, p99 falls to 331us only on the stdlib variant.Kotlin's lead over Rhino goes from ~1.6x to ~2.2x.
Behaviour
Unchanged, and the suite covers the cases that decide it:
"null"vs"\"null\""vsnull, the[]unkown$key that must throw and be swallowed byOnFailure, and quoted literals against field paths.Two equivalences worth stating:
QUOTEDis unanchored becauseRegex.matchesis a full-input match, making the original^/$redundant. Bothwhenbranches were testing the same predicate.removeSurrounding("\"")was not used. It strips only when both quotes are present, whileunwrapstrips each independently, andrawValuecallsunwrapon unquoted strings. A key such as"abcwould resolve differently and change theOnFailureoutcome.Docs
The README Performance section moves to
BENCHMARKS.md, which also records the run-to-run spread per engine. The README keeps the comparison table and a link.