Skip to content

Commit c547cf8

Browse files
authored
perf: extend comprehension scope reuse to builtins, select, lookup (#783)
## Summary - Extend `isNonCapturingBody` to recognize more expression types that produce eager values without capturing the comprehension `ValScope` - This allows the mutable scope reuse fast path to activate for common comprehension patterns like `[std.length(x) for x in arr]` or `[x.field for x in arr]`, avoiding per-iteration `ValScope` allocation via `scope.extendSimple()` + `Arrays.copyOf` ## New expression types covered | Expression | Check | |------------|-------| | `ApplyBuiltin0` | always true (no args) | | `ApplyBuiltin1` | `isNonCapturingBody(a1)` | | `ApplyBuiltin2` | both args non-capturing | | `ApplyBuiltin3` | all 3 args non-capturing | | `ApplyBuiltin4` | all 4 args non-capturing | | `Select` | `isNonCapturingBody(value)` | | `Lookup` | both value and index non-capturing | ## Safety argument The mutable scope reuse path evaluates the body with `visitExpr` (eager). The returned `Val` doesn't capture the comprehension scope. Internal `visitAsLazy` calls within builtins receive non-capturing sub-expressions, so `visitAsLazy` returns eagerly via `tryEagerEval` or direct binding lookup — no `LazyExpr` creation, no stale scope capture. **NOT extended to:** `Apply0-3` (user-defined function calls may create closures that capture scope), `Function` (creates `Val.Func` capturing scope), `ObjBody.MemberList` (creates `Val.Obj` with lazy fields capturing scope). ## Test plan - [x] `./mill 'sjsonnet.jvm[3.3.7]'.test` — all tests pass - [x] `./mill 'sjsonnet.jvm[3.3.7]'.compile` — compiles clean - [x] `isInvariantExpr` (line 478+) already covers the same expression types, confirming this pattern is established
1 parent 9c6debb commit c547cf8

1 file changed

Lines changed: 18 additions & 0 deletions

File tree

sjsonnet/src/sjsonnet/Evaluator.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,24 @@ class Evaluator(
460460
isNonCapturingBody(ie.cond) && isNonCapturingBody(ie.`then`) && isNonCapturingBody(
461461
ie.`else`
462462
)
463+
case ExprTags.ApplyBuiltin0 => true
464+
case ExprTags.ApplyBuiltin1 =>
465+
isNonCapturingBody(e.asInstanceOf[ApplyBuiltin1].a1)
466+
case ExprTags.ApplyBuiltin2 =>
467+
val ab = e.asInstanceOf[ApplyBuiltin2]
468+
isNonCapturingBody(ab.a1) && isNonCapturingBody(ab.a2)
469+
case ExprTags.ApplyBuiltin3 =>
470+
val ab = e.asInstanceOf[ApplyBuiltin3]
471+
isNonCapturingBody(ab.a1) && isNonCapturingBody(ab.a2) && isNonCapturingBody(ab.a3)
472+
case ExprTags.ApplyBuiltin4 =>
473+
val ab = e.asInstanceOf[ApplyBuiltin4]
474+
isNonCapturingBody(ab.a1) && isNonCapturingBody(ab.a2) &&
475+
isNonCapturingBody(ab.a3) && isNonCapturingBody(ab.a4)
476+
case ExprTags.Select =>
477+
isNonCapturingBody(e.asInstanceOf[Select].value)
478+
case ExprTags.Lookup =>
479+
val l = e.asInstanceOf[Lookup]
480+
isNonCapturingBody(l.value) && isNonCapturingBody(l.index)
463481
case _ =>
464482
e.isInstanceOf[Val.Literal]
465483
}

0 commit comments

Comments
 (0)