perf: change Expr from trait to abstract class

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

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.

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

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 {