From 60f149dd00a04fba7dcbc469fc48d991a4aefbd8 Mon Sep 17 00:00:00 2001 From: He-Pin Date: Sun, 19 Apr 2026 05:36:15 +0800 Subject: [PATCH] perf: change Expr from trait to abstract class Move Expr from a trait/interface to an abstract class and make Val extend Expr directly. This keeps literal and function values usable in optimized AST positions while reducing hot-path interface dispatch in the evaluator. Update the README architecture notes so the documented Expr/Val/Eval hierarchy matches the implementation. JMH (bench.runRegressions, 2026-04-26): realistic2 48.139 -> 40.982 ms/op; setUnion 0.603 -> 0.594 ms/op. --- readme.md | 22 +++++++++------- sjsonnet/src/sjsonnet/Expr.scala | 14 +++++++++- sjsonnet/src/sjsonnet/StaticOptimizer.scala | 14 +++++----- sjsonnet/src/sjsonnet/Val.scala | 29 +++++++++++++++------ 4 files changed, 54 insertions(+), 25 deletions(-) diff --git a/readme.md b/readme.md index dd9de86ff..09fad1a93 100644 --- a/readme.md +++ b/readme.md @@ -169,7 +169,7 @@ These three phases are encapsulated in the `sjsonnet.Interpreter` object. Some notes on the values used in parts of the pipeline: -- `sjsonnet.Expr`: this represents `{...}` object literal nodes, `a + b` binary +- `sjsonnet.Expr`: this abstract class represents `{...}` object literal nodes, `a + b` binary operation nodes, `function(a) {...}` definitions and `f(a)` invocations, etc.. Also keeps track of source-offset information so failures can be correlated with line numbers. @@ -182,9 +182,10 @@ Some notes on the values used in parts of the pipeline: raise an error because the first (erroneous) entry of the array is un-used and thus not evaluated. -- Classes representing literals extend `sjsonnet.Val.Literal` which in turn extends - _both_, `Expr` and `Val`. This allows the evaluator to skip over them instead of - having to convert them from one representation to the other. +- `sjsonnet.Val` extends `Expr`, and classes representing literals extend + `sjsonnet.Val.Literal`. This allows the evaluator to skip already-computed values + instead of having to convert them from one representation to the other, while + keeping the hot `Expr` dispatch path on an abstract class hierarchy. ## Performance @@ -214,11 +215,14 @@ The Jsonnet language is _lazy_: expressions don't get evaluated unless their value is needed, and thus even erroneous expressions do not cause a failure if un-used. -Sjsonnet models this with a flat type hierarchy rooted at `sjsonnet.Eval`: +Sjsonnet models this with a flat evaluation hierarchy rooted at `sjsonnet.Eval`, +and a separate `sjsonnet.Expr` abstract class for AST dispatch. `Val` is the bridge: +it is an already-computed `Eval` and also an `Expr` so literals and functions can +appear directly in optimized ASTs. ``` - Eval (trait) — common interface: def value: Val - / \ + Eval (trait) Expr (abstract class) + / \ | Lazy Val (sealed abstract class) (final class) | Val.Str, Val.Num, Val.Arr, Val.Obj, Val.Func, ... @@ -232,8 +236,8 @@ Lazy Val (sealed abstract class) and is thread-safe. After the value is computed, the closure reference is cleared to allow it to be garbage collected. -- **`Val`** represents an already-computed value. It extends `Eval` directly and - implements `value` as simply returning `this`. +- **`Val`** represents an already-computed value. It extends `Eval` and `Expr`, + and implements `value` as simply returning `this`. The hierarchy is intentionally kept flat (only two direct implementors of `Eval`) to enable the JVM JIT compiler's bimorphic inlining optimization on the diff --git a/sjsonnet/src/sjsonnet/Expr.scala b/sjsonnet/src/sjsonnet/Expr.scala index 047b3c13e..2eeb1d22f 100644 --- a/sjsonnet/src/sjsonnet/Expr.scala +++ b/sjsonnet/src/sjsonnet/Expr.scala @@ -10,8 +10,20 @@ import scala.collection.immutable.IntMap * * Each [[Expr]] represents an expression in the Jsonnet program, and contains an integer offset * into the file that is later used to provide error messages. + * + * '''Why `abstract class` and not `trait`?''' This is an intentional JVM performance decision. + * Method calls on a class reference use `invokevirtual` (O(1) vtable lookup), while calls on a + * trait/interface reference use `invokeinterface` (itable search, ~2-5ns slower per call). The + * evaluator calls `e.tag` and pattern-matches on `Expr`-typed references millions of times per + * evaluation; `instanceof` checks against a class hierarchy are also cheaper than against an + * interface (single depth-indexed comparison vs. interface table scan). Since `Expr` does not + * require multiple inheritance — no class mixes in `Expr` alongside an unrelated base class — an + * `abstract class` is strictly faster with no loss of flexibility. + * + * @see + * [[Val]] which extends `Expr` to unify the type hierarchy for dispatch efficiency. */ -trait Expr { +abstract class Expr { /** * Source position of this expression, used for error messages. diff --git a/sjsonnet/src/sjsonnet/StaticOptimizer.scala b/sjsonnet/src/sjsonnet/StaticOptimizer.scala index 4f3384383..b467a2a73 100644 --- a/sjsonnet/src/sjsonnet/StaticOptimizer.scala +++ b/sjsonnet/src/sjsonnet/StaticOptimizer.scala @@ -74,11 +74,11 @@ class StaticOptimizer( case e @ Id(pos, name) => scope.get(name) match { - case ScopedVal(v: Val with Expr, _, _) => v - case ScopedVal(_, _, idx) => ValidId(pos, name, idx) - case null if name == f"$$std" => std - case null if name == "std" => std - case null => + case ScopedVal(v: Val, _, _) => v + case ScopedVal(_, _, idx) => ValidId(pos, name, idx) + case null if name == f"$$std" => std + case null if name == "std" => std + case null => variableResolver(name) match { case Some(v) => v // additional variable resolution case None => @@ -363,8 +363,8 @@ class StaticOptimizer( while (i < target.length) { if (target(i) == null) { params.defaultExprs(i) match { - case v: Val with Expr => target(i) = v - case _ => return null // no default or non-constant + case v: Val => target(i) = v + case _ => return null // no default or non-constant } } i += 1 diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala index c717c93c7..aa9940ff5 100644 --- a/sjsonnet/src/sjsonnet/Val.scala +++ b/sjsonnet/src/sjsonnet/Val.scala @@ -160,14 +160,28 @@ final class LazyDefault( * [[Val]]s represented Jsonnet values that are the result of evaluating a Jsonnet program. The * [[Val]] data structure is essentially a JSON tree, except evaluation of object attributes and * array contents are lazy, and the tree can contain functions. + * + * '''Why `Val extends Expr`?''' Semantically, runtime values and AST nodes are distinct concepts. + * However, certain [[Val]] subtypes ([[Val.Literal]], [[Val.Func]]) already needed to appear in AST + * positions (e.g. constant-folded literals, builtin function references), which previously required + * `Literal extends Val with Expr` / `Func extends Val with Expr`. By lifting the `Expr` parent to + * `Val` itself, we get two concrete benefits: + * + * - '''Unified dispatch:''' The evaluator's `visitExpr(e: Expr)` handles the `case e: Val => e` + * fast path as a pure class hierarchy `instanceof` check (O(1)), rather than a mixed + * class-plus-interface check. + * - '''Simpler hierarchy:''' Eliminates the `with Expr` mixin from `Literal` and `Func`, reducing + * interface table size and keeping the vtable layout compact. + * + * The only cost is that [[TailCall]] (which extends `Val` as a trampoline sentinel) inherits a + * `var pos` field it doesn't conceptually need. This is acceptable because `TailCall` instances are + * transient — created and consumed within a single trampoline loop iteration. */ -sealed abstract class Val extends Eval { +sealed abstract class Val extends Expr with Eval { final def value: Val = this /** Runtime type tag for O(1) dispatch in Materializer (tableswitch). */ private[sjsonnet] def valTag: Byte - - def pos: Position def prettyName: String def cast[T: ClassTag: PrettyNamed]: T = @@ -214,7 +228,7 @@ object Val { private[sjsonnet] final val TAG_OBJ: Byte = 6 private[sjsonnet] final val TAG_FUNC: Byte = 7 - abstract class Literal extends Val with Expr { + abstract class Literal extends Val { final override private[sjsonnet] def tag = ExprTags.`Val.Literal` } abstract class Bool extends Literal { @@ -1547,8 +1561,7 @@ object Val { } abstract class Func(var pos: Position, val defSiteValScope: ValScope, val params: Params) - extends Val - with Expr { + extends Val { final override private[sjsonnet] def tag = ExprTags.`Val.Func` private[sjsonnet] def valTag: Byte = TAG_FUNC @@ -1986,9 +1999,9 @@ final class TailCall( val strict: Boolean = false) extends Val { private[sjsonnet] def valTag: Byte = -1 - def pos: Position = callSiteExpr.pos + var pos: Position = callSiteExpr.pos def prettyName = "tailcall" - def exprErrorString: String = callSiteExpr.exprErrorString + override def exprErrorString: String = callSiteExpr.exprErrorString } object TailCall {