perf(rhino): share sealed standard scope across evaluations - #54
Merged
Conversation
Added engine comparison table, internals explanations, trade-offs, and performance data. Added bench Gradle tasks.
Introduces `reuseContextPerThread` flag to keep one context per thread instead of building and closing per evaluation. Context construction dominated runtime; reusing it removes that cost. Maintains thread-safety and input isolation by injecting a fresh object per call. Adds benchmark comparison, comprehensive docs on trade-offs, and concurrent evaluation tests.
…ntext * origin/main: docs: add benchmark comparison and engine guide (#52) # Conflicts: # README.md # graaljs-evaluator/build.gradle
Build JavaScript standard objects once per evaluator instance and seal them. Each evaluation runs in a cheap child scope with those sealed objects as its prototype, so input data is never shared between evaluations and standard objects are never mutated. Gives ~7.4x speedup while keeping evaluate safe to call concurrently and maintaining input isolation through scope hierarchy.
rapatao
marked this pull request as ready for review
August 31, 2026 17:36
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.
RhinoEvaluator.callrebuilt the entire JavaScript standard library on everyevaluateviainitSafeStandardObjects(), then threw it away. Identical work every call.The standard objects are now built once per evaluator instance and sealed. Each evaluation gets a cheap child scope with those objects as its prototype, and the input is injected there. This is the usual Rhino shared-scope pattern.
No new constructor parameter and no
ThreadLocal. Unlike the GraalJS context reuse in #52, this loses no isolation, so it is not opt-in.Results
./gradlew :rhino-evaluator:bench -PbenchIterations=2000, 147-expression suite:7.9x, stable between 344,000 and 353,000 across three runs. Rhino moves from ~12x the Kotlin engine's cost to ~1.6x, ahead of GraalJS in either mode.
Isolating a single rule in a warmed loop, scope setup drops from 13.00us to 0.17us (~63% of the call to ~14%).
Isolation
The child scope keeps all three properties the risk analysis called out:
parentScope = nullmakes the child the top of the scope chain, so a global assigned by a rule dies with the evaluationTwo implementation details worth knowing.
ScriptableObject.getTopScopeValuewalks the prototype chain, so theClassCacheand library scope associated with the shared scope stay reachable from the child; sharing theClassCachealso meansTypedInjector's reflection results now cache across evaluations. And the shared scope object itself must not be sealed withsealObject(), only its builtins, becauseinitSafeStandardObjectsregistersLazilyLoadedCtorentries that still write themselves onto the scope on first access.Documentation corrections
The README claim that
interpretedMode = falseis "about 10x slower" no longer holds. Compiled mode is unchanged, but the interpreted path got ~10x faster, so the gap is now about 114x (37.7ms vs 0.33ms per iteration). Corrected in both places it appeared.The "Where the time goes" table was re-measured for all engines with a single harness. Its
one evaluate callcolumn had been under-warmed for the cheap engines, and updating only the Rhino row would have made Rhino appear faster than the Kotlin engine on a single rule, contradicting the suite table. The GraalJS row moves from 122.4us to 133.7us; that reflects the per-call context close added in #52, not this change.Tests
RhinoSharedScopeTestcovers stale bindings between evaluations, rule-defined globals not leaking, sealed builtins rejectingArray.prototype.foo = 1, and 8-thread concurrent evaluation of the full case set../gradlew buildgreen, 177 tests.