From 73cbedac64bb38aab03695fef732baf474d75ddc Mon Sep 17 00:00:00 2001 From: TristonianJones Date: Mon, 14 Sep 2026 16:21:01 -0700 Subject: [PATCH] Rearchitect ExecutionFrame to be a direct Activation --- cel/library.go | 4 +- cel/program.go | 9 +- ext/bindings.go | 84 +++++---- interpreter/activation.go | 14 +- interpreter/async.go | 2 +- interpreter/attributes.go | 21 ++- interpreter/decorators.go | 14 +- interpreter/frame.go | 277 ++++++++++++++---------------- interpreter/frame_test.go | 6 +- interpreter/interpretable.go | 117 ++++++++----- interpreter/interpreter.go | 7 +- interpreter/interpreter_test.go | 60 ++----- interpreter/runtimecost.go | 2 +- interpreter/runtimecost_test.go | 10 +- interpreter/runtimememory.go | 4 +- interpreter/runtimememory_test.go | 10 +- 16 files changed, 339 insertions(+), 302 deletions(-) diff --git a/cel/library.go b/cel/library.go index 9f1885838..66acd772e 100644 --- a/cel/library.go +++ b/cel/library.go @@ -981,7 +981,7 @@ func (opt *evalOptionalOr) Exec(frame *interpreter.ExecutionFrame) ref.Val { // Eval evaluates the left-hand side optional to determine whether it contains a value, else // proceeds with the right-hand side evaluation. func (opt *evalOptionalOr) Eval(ctx interpreter.Activation) ref.Val { - return opt.Exec(interpreter.AsFrame(ctx)) + return interpreter.EvalActivation(ctx, opt.Exec) } // evalOptionalOrValue selects between an optional or a concrete value. If the optional has a value, @@ -1017,7 +1017,7 @@ func (opt *evalOptionalOrValue) Exec(frame *interpreter.ExecutionFrame) ref.Val // Eval evaluates the left-hand side optional to determine whether it contains a value, else // proceeds with the right-hand side evaluation. func (opt *evalOptionalOrValue) Eval(ctx interpreter.Activation) ref.Val { - return opt.Exec(interpreter.AsFrame(ctx)) + return interpreter.EvalActivation(ctx, opt.Exec) } type timeLegacyLibrary struct{} diff --git a/cel/program.go b/cel/program.go index 5027706e5..4f6284a9a 100644 --- a/cel/program.go +++ b/cel/program.go @@ -426,6 +426,7 @@ func (p *prog) Eval(input any) (out ref.Val, det *EvalDetails, err error) { } // Build a hierarchical activation if there are default vars set. var frame *interpreter.ExecutionFrame + var mustClose bool if f, ok := input.(*interpreter.ExecutionFrame); ok { frame = f } else { @@ -433,7 +434,7 @@ func (p *prog) Eval(input any) (out ref.Val, det *EvalDetails, err error) { if err != nil { return nil, nil, err } - defer frame.Close() + mustClose = true } // Configure error recovery and details capture for evaluation. defer func() { @@ -446,6 +447,9 @@ func (p *prog) Eval(input any) (out ref.Val, det *EvalDetails, err error) { err = fmt.Errorf("internal error: %v", r) } } + if mustClose { + frame.Close() + } }() if p.observable != nil { @@ -499,8 +503,7 @@ func (p *prog) newExecutionFrame(input any) (*interpreter.ExecutionFrame, error) return nil, err } if p.defaultVars != nil { - // Update the frame's activation in place. - frame.Activation = interpreter.NewHierarchicalActivation(p.defaultVars, frame.Activation) + frame.SetDefaultVars(p.defaultVars) } return frame, nil diff --git a/ext/bindings.go b/ext/bindings.go index 1bf97c59c..33ab2b466 100644 --- a/ext/bindings.go +++ b/ext/bindings.go @@ -123,6 +123,9 @@ func (lib *celBindings) ProgramOptions() []cel.ProgramOption { // Non-empty block if block, ok := args[0].(interpreter.InterpretableConstructor); ok { slotExprs := block.InitVals() + if len(slotExprs) == 0 { + return expr, nil + } return newDynamicBlock(slotExprs, expr), nil } // Constant valued block which can happen during runtime optimization. @@ -228,18 +231,15 @@ func (b *dynamicBlock) ID() int64 { func (b *dynamicBlock) Exec(frame *interpreter.ExecutionFrame) ref.Val { sa := b.slotActivationPool.Get().(*dynamicSlotActivation) sa.frame = frame.Push(sa) - // Ensure the 'unwrapped' Activation points to the original one from the frame, - // and not the hierarchical activation which composes the original and the slot - // activation. - sa.Activation = frame.Activation - defer sa.frame.Pop() - defer b.clearSlots(sa) - return b.expr.Exec(sa.frame) + res := b.expr.Exec(sa.frame) + sa.frame.Pop() + b.clearSlots(sa) + return res } // Eval implements the Interpretable interface method. func (b *dynamicBlock) Eval(activation cel.Activation) ref.Val { - return b.Exec(interpreter.AsFrame(activation)) + return interpreter.EvalActivation(activation, b.Exec) } func (b *dynamicBlock) clearSlots(sa *dynamicSlotActivation) { @@ -253,16 +253,32 @@ type slotVal struct { } type dynamicSlotActivation struct { - cel.Activation frame *interpreter.ExecutionFrame slotExprs []interpreter.InterpretableV2 slotCount int slotVals []*slotVal } -// Unwrap returns the underlying activation. +// Unwrap returns the underlying activation outside the block. func (sa *dynamicSlotActivation) Unwrap() cel.Activation { - return sa.Activation + if sa.frame != nil { + return sa.frame.Parent() + } + return nil +} + +// Parent returns the parent activation outside the block. +func (sa *dynamicSlotActivation) Parent() cel.Activation { + if sa.frame != nil { + return sa.frame.Parent() + } + return nil +} + +// IsLocalVariable reports whether the variable is locally bound to a block slot. +func (sa *dynamicSlotActivation) IsLocalVariable(name string) bool { + _, found := matchSlot(name, sa.slotCount) + return found } // ResolveName implements the Activation interface method but handles variables prefixed with `@index` @@ -283,11 +299,10 @@ func (sa *dynamicSlotActivation) ResolveName(name string) (any, bool) { v.value = &val return val, true } - return sa.Activation.ResolveName(name) + return nil, false } func (sa *dynamicSlotActivation) reset() { - sa.Activation = nil sa.frame = nil for _, sv := range sa.slotVals { sv.visited = false @@ -311,40 +326,39 @@ func (b *constantBlock) ID() int64 { return b.expr.ID() } -// Exec implements the Interpretable interface method and pushes a new frame onto the -// stack for the duration of the block execution. +// Exec implements the Interpretable interface method and evaluates the block with +// constant slot resolution using a child execution frame scoped to the constant slots. func (b *constantBlock) Exec(frame *interpreter.ExecutionFrame) ref.Val { - sa := constantSlotActivation{Activation: frame.Activation, slots: b.slots, slotCount: b.slotCount} - sa.frame = frame.Push(sa) - defer sa.frame.Pop() - return b.expr.Exec(sa.frame) + child := frame.Push(b) + res := b.expr.Exec(child) + child.Pop() + return res } // Eval implements the interpreter.Interpretable interface method, and will proxy @index prefixed variable // lookups into a set of constant slots determined from the plan step. func (b *constantBlock) Eval(activation cel.Activation) ref.Val { - return b.Exec(interpreter.AsFrame(activation)) + return interpreter.EvalActivation(activation, b.Exec) } -type constantSlotActivation struct { - cel.Activation - frame *interpreter.ExecutionFrame - slots traits.Lister - slotCount int +// ResolveName implements Activation interface method and proxies @index prefixed lookups into the slot +// activation associated with the block scope. +func (b *constantBlock) ResolveName(name string) (any, bool) { + if idx, found := matchSlot(name, b.slotCount); found { + return b.slots.Get(types.Int(idx)), true + } + return nil, false } -// Unwrap returns the underlying activation. -func (sa *constantSlotActivation) Unwrap() cel.Activation { - return sa.Activation +// Parent implements the Activation interface. +func (b *constantBlock) Parent() cel.Activation { + return nil } -// ResolveName implements Activation interface method and proxies @index prefixed lookups into the slot -// activation associated with the block scope. -func (sa constantSlotActivation) ResolveName(name string) (any, bool) { - if idx, found := matchSlot(name, sa.slotCount); found { - return sa.slots.Get(types.Int(idx)), true - } - return sa.Activation.ResolveName(name) +// IsLocalVariable reports whether the variable is locally bound to a block slot. +func (b *constantBlock) IsLocalVariable(name string) bool { + _, found := matchSlot(name, b.slotCount) + return found } func matchSlot(name string, slotCount int) (int, bool) { diff --git a/interpreter/activation.go b/interpreter/activation.go index 15cbd3002..981693a1c 100644 --- a/interpreter/activation.go +++ b/interpreter/activation.go @@ -43,7 +43,8 @@ func EmptyActivation() Activation { type emptyActivation struct{} func (emptyActivation) ResolveName(string) (any, bool) { return nil, false } -func (emptyActivation) Parent() Activation { return nil } + +func (emptyActivation) Parent() Activation { return nil } // NewActivation returns an activation based on a map-based binding where the map keys are // expected to be qualified names used with ResolveName calls. @@ -122,10 +123,15 @@ func (a *hierarchicalActivation) Parent() Activation { // ResolveName implements the Activation interface method. func (a *hierarchicalActivation) ResolveName(name string) (any, bool) { - if object, found := a.child.ResolveName(name); found { - return object, found + if a.child != nil { + if object, found := a.child.ResolveName(name); found { + return object, found + } } - return a.parent.ResolveName(name) + if a.parent != nil { + return a.parent.ResolveName(name) + } + return nil, false } // Unwrap returns the parent activation, stripping the local child scope. diff --git a/interpreter/async.go b/interpreter/async.go index a169a114c..9494167f5 100644 --- a/interpreter/async.go +++ b/interpreter/async.go @@ -93,7 +93,7 @@ func (fn *evalAsyncFunc) Args() []InterpretableV2 { // Eval implements the Interpretable interface method. func (fn *evalAsyncFunc) Eval(vars Activation) ref.Val { - return fn.Exec(AsFrame(vars)) + return EvalActivation(vars, fn.Exec) } // Exec implements the InterpretableV2 interface method. diff --git a/interpreter/attributes.go b/interpreter/attributes.go index c92e13365..c1f940cad 100644 --- a/interpreter/attributes.go +++ b/interpreter/attributes.go @@ -335,7 +335,11 @@ func (a *absoluteAttribute) Resolve(vars Activation) (any, error) { inputVars = vars wrapped, ok := inputVars.(activationWrapper) for ok { - inputVars = wrapped.Unwrap() + next := wrapped.Unwrap() + if next == nil { + break + } + inputVars = next wrapped, ok = inputVars.(activationWrapper) } } @@ -1304,6 +1308,12 @@ func (q *unknownQualifier) Value() ref.Val { } func applyQualifiers(vars Activation, obj any, qualifiers []Qualifier) (any, bool, error) { + // Early return if there's nothing to apply + if len(qualifiers) == 0 { + return obj, false, nil + } + + // Test for an optional select that's on an empty value. optObj, isOpt := obj.(*types.Optional) if isOpt { if !optObj.HasValue() { @@ -1312,6 +1322,7 @@ func applyQualifiers(vars Activation, obj any, qualifiers []Qualifier) (any, boo obj = optObj.GetValue() } + frame, hasFrame := vars.(*ExecutionFrame) var err error for _, qual := range qualifiers { var qualObj any @@ -1319,7 +1330,9 @@ func applyQualifiers(vars Activation, obj any, qualifiers []Qualifier) (any, boo if isOpt { var present bool qualObj, present, err = qual.QualifyIfPresent(vars, obj, false) - trackCostQualify(AsFrame(vars), qual.ID()) + if hasFrame { + trackCostQualify(frame, qual.ID()) + } if err != nil { return nil, false, err } @@ -1331,7 +1344,9 @@ func applyQualifiers(vars Activation, obj any, qualifiers []Qualifier) (any, boo } } else { qualObj, err = qual.Qualify(vars, obj) - trackCostQualify(AsFrame(vars), qual.ID()) + if hasFrame { + trackCostQualify(frame, qual.ID()) + } if err != nil { return nil, false, err } diff --git a/interpreter/decorators.go b/interpreter/decorators.go index 7402d18fa..2b193dbb8 100644 --- a/interpreter/decorators.go +++ b/interpreter/decorators.go @@ -251,7 +251,9 @@ func maybeOptimizeConstUnary(i InterpretableV2, call InterpretableCall) (Interpr if !isConst { return i, nil } - val := call.Eval(EmptyActivation()) + frame, _ := NewExecutionFrame(EmptyActivation()) + val := call.Exec(frame) + frame.Close() if types.IsError(val) { return nil, val.(*types.Err) } @@ -265,7 +267,10 @@ func maybeBuildListLiteral(i InterpretableV2, l *evalList) (InterpretableV2, err return i, nil } } - return NewConstValue(l.ID(), l.Eval(EmptyActivation())), nil + frame, _ := NewExecutionFrame(EmptyActivation()) + constVal := l.Exec(frame) + frame.Close() + return NewConstValue(l.ID(), constVal), nil } func maybeBuildMapLiteral(i InterpretableV2, mp *evalMap) (InterpretableV2, error) { @@ -279,7 +284,10 @@ func maybeBuildMapLiteral(i InterpretableV2, mp *evalMap) (InterpretableV2, erro return i, nil } } - return NewConstValue(mp.ID(), mp.Eval(EmptyActivation())), nil + frame, _ := NewExecutionFrame(EmptyActivation()) + constVal := mp.Exec(frame) + frame.Close() + return NewConstValue(mp.ID(), constVal), nil } // maybeOptimizeSetMembership may convert an 'in' operation against a list to map key membership diff --git a/interpreter/frame.go b/interpreter/frame.go index ff8191154..e01dae860 100644 --- a/interpreter/frame.go +++ b/interpreter/frame.go @@ -73,25 +73,60 @@ type evalContext struct { // The execution frame must not be stored in any fashion as its lifecycle is completely // controlled by the CEL evaluation process. type ExecutionFrame struct { - // Activation provides the context for resolving variables by name. - Activation - - // parent provides the context for parent scopes (used for comprehension iterators). + // parent provides the context for parent scopes (used for comprehension iterators and nested blocks). parent *ExecutionFrame + // scope provides the local activation for this frame (e.g. comprehension folder, block slots, or input Activation). + scope Activation + + // vars holds map-based input variables directly on the frame to eliminate pool allocations. + vars map[string]any + + // lazyVars caches evaluations of lazy variables (func() any / func() ref.Val) in vars. + lazyVars map[string]any + // ctx provides the shared evaluation state across frames. ctx *evalContext + + // costTracker provides direct access to the active CostTracker for this evaluation pass. + costTracker *CostTracker +} + +// Scope returns the local activation scope for this frame, if present. +func (f *ExecutionFrame) Scope() Activation { + return f.scope +} + +// SetScope sets the local activation scope for this frame. +func (f *ExecutionFrame) SetScope(scope Activation) { + f.scope = scope +} + +// SetDefaultVars sets the default variables activation for the frame, composing it +// with any existing scope activation. +func (f *ExecutionFrame) SetDefaultVars(defaultVars Activation) { + if defaultVars == nil { + return + } + if f.scope != nil { + f.scope = NewHierarchicalActivation(defaultVars, f.scope) + } else { + f.scope = defaultVars + } } // NewExecutionFrame creates a new execution frame from the pool. func NewExecutionFrame(input any) (*ExecutionFrame, error) { f := frameStack.Get().(*ExecutionFrame) switch v := input.(type) { + case emptyActivation: + // empty frame, no backing scope needed case Activation: - f.Activation = v + f.scope = v case map[string]any: - f.Activation = activationInput.create(v) + f.vars = v default: + frameStack.Put(f) return nil, fmt.Errorf("invalid input, wanted Activation or map[string]any, got: (%T)%v", input, input) } return f, nil @@ -139,30 +174,25 @@ func (f *ExecutionFrame) Close() { } f.ctx = nil f.parent = nil - if f.Activation != nil { - switch a := f.Activation.(type) { - case *hierarchicalActivation: - if child, ok := a.child.(*inputActivation); ok { - activationInput.release(child) - } - activationStack.release(a) - case *inputActivation: - activationInput.release(a) - } - f.Activation = nil - frameStack.Put(f) + f.costTracker = nil + if f.vars != nil { + f.vars = nil + clear(f.lazyVars) } + f.scope = nil + frameStack.Put(f) } // Push pushes the given activation onto the activation stack and returns the new frame. // // This operation is internal to the interpreter and is used to handle comprehension -// scoping. The child frame inherits the shared evalContext from the parent. +// and block scoping. The child frame inherits the shared evalContext from the parent. func (f *ExecutionFrame) Push(activation Activation) *ExecutionFrame { child := frameStack.Get().(*ExecutionFrame) child.parent = f child.ctx = f.ctx - child.Activation = activationStack.create(f.Activation, activation) + child.costTracker = f.costTracker + child.scope = activation return child } @@ -172,37 +202,105 @@ func (f *ExecutionFrame) Pop() *ExecutionFrame { return f } parent := f.parent - activationStack.release(f.Activation) - f.Activation = nil + f.scope = nil f.parent = nil f.ctx = nil + f.costTracker = nil + if f.vars != nil { + f.vars = nil + clear(f.lazyVars) + } frameStack.Put(f) return parent } // ResolveName implements the Activation interface by proxying to the internal activation. func (f *ExecutionFrame) ResolveName(name string) (any, bool) { - return f.Activation.ResolveName(name) + if f.vars != nil { + v, found := f.vars[name] + if found { + if f.lazyVars != nil { + if resolved, found := f.lazyVars[name]; found { + return resolved, true + } + } + var lazy any + switch obj := v.(type) { + case func() ref.Val: + lazy = obj() + case func() any: + lazy = obj() + default: + return obj, true + } + if f.lazyVars == nil { + f.lazyVars = make(map[string]any, 4) + } + f.lazyVars[name] = lazy + return lazy, true + } + } + if f.scope != nil { + if val, found := f.scope.ResolveName(name); found { + return val, true + } + } + if f.parent != nil { + return f.parent.ResolveName(name) + } + return nil, false } -// Parent implements the Activation interface by proxying to the internal activation. +// Parent implements the Activation interface by proxying to the parent frame or scope. func (f *ExecutionFrame) Parent() Activation { - return f.Activation.Parent() + if f.parent != nil { + if f.parent.scope != nil { + return f.parent.scope + } + return f.parent + } + if f.scope != nil { + return f.scope.Parent() + } + return nil } -// AsPartialActivation implements the PartialActivation interface by proxying to the internal activation. +// AsPartialActivation implements the PartialActivation interface by proxying to the internal scope or parent. func (f *ExecutionFrame) AsPartialActivation() (PartialActivation, bool) { - return AsPartialActivation(f.Activation) + if f.scope != nil { + if pa, ok := AsPartialActivation(f.scope); ok { + return pa, true + } + } + if f.parent != nil { + return f.parent.AsPartialActivation() + } + return nil, false +} + +// UnknownAttributePatterns implements the PartialActivation interface returning the unknown patterns +// if they were provided to the input activation, or an empty set if the frame is not partial. +func (f *ExecutionFrame) UnknownAttributePatterns() []*AttributePattern { + if pa, ok := f.AsPartialActivation(); ok { + return pa.UnknownAttributePatterns() + } + return []*AttributePattern{} } -// Unwrap returns the internal activation. +// Unwrap returns the local activation scope if present, or the parent frame. func (f *ExecutionFrame) Unwrap() Activation { - return f.Activation + if f.scope != nil { + return f.scope + } + if f.parent != nil { + return f.parent + } + return nil } // IsLocalVariable reports whether the variable name is locally bound in the frame. func (f *ExecutionFrame) IsLocalVariable(name string) bool { - if holder, ok := f.Activation.(localVariableHolder); ok { + if holder, ok := f.scope.(localVariableHolder); ok { if holder.IsLocalVariable(name) { return true } @@ -237,10 +335,10 @@ func (f *ExecutionFrame) CheckInterrupt() bool { // CostTracker returns the active CostTracker for this evaluation pass, or nil. func (f *ExecutionFrame) CostTracker() *CostTracker { - if f.ctx == nil { + if f == nil { return nil } - return f.ctx.costs + return f.costTracker } // SetCostTracker sets the active CostTracker for this evaluation pass. @@ -249,6 +347,7 @@ func (f *ExecutionFrame) SetCostTracker(tracker *CostTracker) { f.ctx = evalContextPool.Get().(*evalContext) } f.ctx.costs = tracker + f.costTracker = tracker } // ComputeResult tracks and computes the result of the given asynchronous function. @@ -349,117 +448,3 @@ var evalContextPool = &sync.Pool{ return &evalContext{} }, } - -type activationStackPool struct { - sync.Pool -} - -func (pool *activationStackPool) create(parent, child Activation) Activation { - h := pool.Get().(*hierarchicalActivation) - h.child = child - h.parent = parent - h.poolAllocated = true - return h -} - -func (pool *activationStackPool) release(activation Activation) { - h, ok := activation.(*hierarchicalActivation) - if !ok || !h.poolAllocated { - return - } - h.parent = nil - h.child = nil - pool.Pool.Put(h) -} - -func newActivationStackPool() *activationStackPool { - return &activationStackPool{ - Pool: sync.Pool{ - New: func() any { - return &hierarchicalActivation{} - }, - }, - } -} - -type inputActivation struct { - vars map[string]any - lazyVars map[string]any -} - -// ResolveName looks up the value of the input variable name, if found. -// -// Lazy bindings may be supplied within the map-based input in either of the following forms: -// - func() any -// - func() ref.Val -// -// The lazy binding will only be invoked once per evaluation. -// -// Values which are not represented as ref.Val types on input may be adapted to a ref.Val using -// the types.Adapter configured in the environment. -func (a *inputActivation) ResolveName(name string) (any, bool) { - v, found := a.vars[name] - if !found { - return nil, false - } - switch obj := v.(type) { - case func() ref.Val: - if resolved, found := a.lazyVars[name]; found { - return resolved, true - } - lazy := obj() - a.lazyVars[name] = lazy - return lazy, true - case func() any: - if resolved, found := a.lazyVars[name]; found { - return resolved, true - } - lazy := obj() - a.lazyVars[name] = lazy - return lazy, true - default: - return obj, true - } -} - -// Parent implements the Activation interface -func (a *inputActivation) Parent() Activation { - return nil -} - -func newActivationInputPool() *activationInputPool { - return &activationInputPool{ - Pool: sync.Pool{ - New: func() any { - return &inputActivation{ - lazyVars: make(map[string]any), - } - }, - }, - } -} - -type activationInputPool struct { - sync.Pool -} - -// create initializes a pooled Activation object with the map input. -func (p *activationInputPool) create(vars map[string]any) *inputActivation { - a := p.Pool.Get().(*inputActivation) - a.vars = vars - return a -} - -func (p *activationInputPool) release(value any) { - a := value.(*inputActivation) - for k := range a.lazyVars { - delete(a.lazyVars, k) - } - a.vars = nil - p.Pool.Put(a) -} - -var ( - activationStack = newActivationStackPool() - activationInput = newActivationInputPool() -) diff --git a/interpreter/frame_test.go b/interpreter/frame_test.go index a1bb7bbf6..c0c914a11 100644 --- a/interpreter/frame_test.go +++ b/interpreter/frame_test.go @@ -265,8 +265,8 @@ func TestFrameUnwrap(t *testing.T) { childFrame := frame.Push(childAct) defer childFrame.Pop() - if got := childFrame.Unwrap(); got != childFrame.Activation { - t.Errorf("Unwrap() got %v, want %v", got, childFrame.Activation) + if got := childFrame.Unwrap(); got != childFrame.Scope() { + t.Errorf("Unwrap() got %v, want %v", got, childFrame.Scope()) } } @@ -410,7 +410,7 @@ func TestFrameLifecycleAndPooling(t *testing.T) { if err != nil { t.Fatalf("NewActivation failed: %v", err) } - frame.Activation = NewHierarchicalActivation(parentAct, frame.Activation) + frame.SetDefaultVars(parentAct) val, found = frame.ResolveName("c") if !found || val != 3 { diff --git a/interpreter/interpretable.go b/interpreter/interpretable.go index f177ade4d..f49443ba3 100644 --- a/interpreter/interpretable.go +++ b/interpreter/interpretable.go @@ -161,7 +161,7 @@ func (oi *ObservableInterpretable) Exec(frame *ExecutionFrame) ref.Val { // Eval proxies to the ObserveEval method while invoking a no-op callback to report the observations. func (oi *ObservableInterpretable) Eval(vars Activation) ref.Val { - return oi.ObserveExec(AsFrame(vars), func(any) {}) + return oi.ObserveEval(vars, func(any) {}) } // ObserveEval evaluates an interpretable and performs per-evaluation state-tracking. @@ -169,7 +169,16 @@ func (oi *ObservableInterpretable) Eval(vars Activation) ref.Val { // This method is concurrency safe and the expectation is that the observer function will use // a switch statement to determine the type of the state which has been reported back from the call. func (oi *ObservableInterpretable) ObserveEval(vars Activation, observer func(any)) ref.Val { - return oi.ObserveExec(AsFrame(vars), observer) + frame, ok := vars.(*ExecutionFrame) + if !ok { + var err error + frame, err = NewExecutionFrame(vars) + if err != nil { + return types.NewErr("invalid activation: %v", err) + } + defer frame.Close() + } + return oi.ObserveExec(frame, observer) } // ObserveExec evaluates an interpretable and performs per-evaluation state-tracking. @@ -195,18 +204,19 @@ func (oi *ObservableInterpretable) ObserveExec(frame *ExecutionFrame, observer f return result } -// AsFrame promotes an Activation to an ExecutionFrame. -func AsFrame(a Activation) *ExecutionFrame { - if f, ok := a.(*ExecutionFrame); ok { - return f +// EvalActivation runs an InterpretableV2 Exec method safely with any Activation, +// wrapping it in a pooled ExecutionFrame if needed and releasing it after execution. +func EvalActivation(ctx Activation, exec func(*ExecutionFrame) ref.Val) ref.Val { + if f, ok := ctx.(*ExecutionFrame); ok { + return exec(f) } - frame := &ExecutionFrame{Activation: a} - // Walk the activation hierarchy to find a parent ExecutionFrame and inherit - // its shared context. - if parent := findFrame(a); parent != nil { - frame.ctx = parent.ctx + f, err := NewExecutionFrame(ctx) + if err != nil { + return types.NewErr("invalid activation: %v", err) } - return frame + res := exec(f) + f.Close() + return res } // findFrame walks the activation hierarchy via Unwrap and Parent to locate an @@ -258,7 +268,7 @@ func (test *evalTestOnly) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (test *evalTestOnly) Eval(ctx Activation) ref.Val { - return test.Exec(AsFrame(ctx)) + return EvalActivation(ctx, test.Exec) } // AddQualifier appends a qualifier that will always and only perform a presence test. @@ -376,7 +386,7 @@ func (or *evalOr) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (or *evalOr) Eval(ctx Activation) ref.Val { - return or.Exec(AsFrame(ctx)) + return EvalActivation(ctx, or.Exec) } type evalAnd struct { @@ -424,7 +434,7 @@ func (and *evalAnd) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (and *evalAnd) Eval(ctx Activation) ref.Val { - return and.Exec(AsFrame(ctx)) + return EvalActivation(ctx, and.Exec) } type evalEq struct { @@ -468,7 +478,7 @@ func (eq *evalEq) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (eq *evalEq) Eval(ctx Activation) ref.Val { - return eq.Exec(AsFrame(ctx)) + return EvalActivation(ctx, eq.Exec) } // Function implements the InterpretableCall interface method. @@ -527,7 +537,7 @@ func (ne *evalNe) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (ne *evalNe) Eval(ctx Activation) ref.Val { - return ne.Exec(AsFrame(ctx)) + return EvalActivation(ctx, ne.Exec) } // Function implements the InterpretableCall interface method. @@ -566,7 +576,7 @@ func (zero *evalZeroArity) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (zero *evalZeroArity) Eval(ctx Activation) ref.Val { - return zero.Exec(AsFrame(ctx)) + return EvalActivation(ctx, zero.Exec) } // Function implements the InterpretableCall interface method. @@ -634,7 +644,7 @@ func (un *evalUnary) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (un *evalUnary) Eval(ctx Activation) ref.Val { - return un.Exec(AsFrame(ctx)) + return EvalActivation(ctx, un.Exec) } // Function implements the InterpretableCall interface method. @@ -706,7 +716,7 @@ func (bin *evalBinary) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (bin *evalBinary) Eval(ctx Activation) ref.Val { - return bin.Exec(AsFrame(ctx)) + return EvalActivation(ctx, bin.Exec) } // Function implements the InterpretableCall interface method. @@ -797,7 +807,7 @@ func (fn *evalVarArgs) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (fn *evalVarArgs) Eval(ctx Activation) ref.Val { - return fn.Exec(AsFrame(ctx)) + return EvalActivation(ctx, fn.Exec) } // Function implements the InterpretableCall interface method. @@ -865,7 +875,7 @@ func (l *evalList) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (l *evalList) Eval(ctx Activation) ref.Val { - return l.Exec(AsFrame(ctx)) + return EvalActivation(ctx, l.Exec) } func (l *evalList) InitVals() []InterpretableV2 { @@ -931,7 +941,7 @@ func (m *evalMap) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (m *evalMap) Eval(ctx Activation) ref.Val { - return m.Exec(AsFrame(ctx)) + return EvalActivation(ctx, m.Exec) } func (m *evalMap) InitVals() []InterpretableV2 { @@ -1003,7 +1013,7 @@ func (o *evalObj) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (o *evalObj) Eval(ctx Activation) ref.Val { - return o.Exec(AsFrame(ctx)) + return EvalActivation(ctx, o.Exec) } // InitVals implements the InterpretableConstructor interface method. @@ -1042,14 +1052,23 @@ func (fold *evalFold) ID() int64 { // Exec implements the InterpretableV2 interface method. func (fold *evalFold) Exec(frame *ExecutionFrame) ref.Val { - // Initialize the folder interface f := newFolder(fold, frame) - defer releaseFolder(f) - foldRange := fold.iterRange.Exec(frame) + + // Early return if the folder is an error or unknown. if types.IsUnknownOrError(foldRange) { + releaseFolder(f) return foldRange } + + // Short-circuit the iteration if the range is empty. + if sizer, ok := foldRange.(traits.Sizer); ok && sizer.Size() == types.IntZero { + res := f.evalResult() + releaseFolder(f) + return res + } + + // Otherwise, attempt a two variable fold. if fold.iterVar2 != "" { var foldable traits.Foldable switch r := foldRange.(type) { @@ -1063,19 +1082,25 @@ func (fold *evalFold) Exec(frame *ExecutionFrame) ref.Val { return types.NewErrWithNodeID(fold.ID(), "unsupported comprehension range type: %T", foldRange) } foldable.Fold(f) - return f.evalResult() + res := f.evalResult() + releaseFolder(f) + return res } + // If the value is not foldable or just a single variable, fallback to an iterable fold. if !foldRange.Type().HasTrait(traits.IterableType) { + releaseFolder(f) return types.ValOrErr(foldRange, "got '%T', expected iterable type", foldRange) } iterable := foldRange.(traits.Iterable) - return f.foldIterable(iterable) + res := f.foldIterable(iterable) + releaseFolder(f) + return res } // Eval implements the Interpretable interface method. func (fold *evalFold) Eval(ctx Activation) ref.Val { - return fold.Exec(AsFrame(ctx)) + return EvalActivation(ctx, fold.Exec) } // Optional Interpretable implementations that specialize, subsume, or extend the core evaluation @@ -1108,7 +1133,7 @@ func (e *evalSetMembership) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (e *evalSetMembership) Eval(ctx Activation) ref.Val { - return e.Exec(AsFrame(ctx)) + return EvalActivation(ctx, e.Exec) } // evalWatch is an Interpretable implementation that wraps the execution of a given @@ -1127,7 +1152,7 @@ func (e *evalWatch) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (e *evalWatch) Eval(vars Activation) ref.Val { - return e.Exec(AsFrame(vars)) + return EvalActivation(vars, e.Exec) } // evalWatchAttr describes a watcher of an InterpretableAttribute Interpretable. @@ -1191,7 +1216,7 @@ func (e *evalWatchAttr) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (e *evalWatchAttr) Eval(vars Activation) ref.Val { - return e.Exec(AsFrame(vars)) + return EvalActivation(vars, e.Exec) } // evalWatchConstQual observes the qualification of an object using a constant boolean, int, @@ -1327,7 +1352,7 @@ func (e *evalWatchConst) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (e *evalWatchConst) Eval(vars Activation) ref.Val { - return e.Exec(AsFrame(vars)) + return EvalActivation(vars, e.Exec) } // evalExhaustiveOr is just like evalOr, but does not short-circuit argument evaluation. @@ -1379,7 +1404,7 @@ func (or *evalExhaustiveOr) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (or *evalExhaustiveOr) Eval(ctx Activation) ref.Val { - return or.Exec(AsFrame(ctx)) + return EvalActivation(ctx, or.Exec) } // evalExhaustiveAnd is just like evalAnd, but does not short-circuit argument evaluation. @@ -1431,7 +1456,7 @@ func (and *evalExhaustiveAnd) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (and *evalExhaustiveAnd) Eval(ctx Activation) ref.Val { - return and.Exec(AsFrame(ctx)) + return EvalActivation(ctx, and.Exec) } // evalExhaustiveConditional is like evalConditional, but does not short-circuit argument @@ -1470,7 +1495,7 @@ func (cond *evalExhaustiveConditional) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (cond *evalExhaustiveConditional) Eval(ctx Activation) ref.Val { - return cond.Exec(AsFrame(ctx)) + return EvalActivation(ctx, cond.Exec) } // evalAttr evaluates an Attribute value. @@ -1521,7 +1546,7 @@ func (a *evalAttr) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable interface method. func (a *evalAttr) Eval(ctx Activation) ref.Val { - return a.Exec(AsFrame(ctx)) + return EvalActivation(ctx, a.Exec) } // Qualify proxies to the Attribute's Qualify method. @@ -1572,7 +1597,7 @@ func (c *evalWatchConstructor) Exec(frame *ExecutionFrame) ref.Val { // Eval implements the Interpretable Eval function. func (c *evalWatchConstructor) Eval(vars Activation) ref.Val { - return c.Exec(AsFrame(vars)) + return EvalActivation(vars, c.Exec) } func invalidOptionalEntryInit(field any, value ref.Val) ref.Val { @@ -1693,10 +1718,16 @@ func (f *folder) ResolveName(name string) (any, bool) { } if !f.computeResult { if name == f.iterVar { + if v, ok := f.iterVar1Val.(ref.Val); ok { + return v, true + } f.iterVar1Val = f.adapter.NativeToValue(f.iterVar1Val) return f.iterVar1Val, true } if name == f.iterVar2 { + if v, ok := f.iterVar2Val.(ref.Val); ok { + return v, true + } f.iterVar2Val = f.adapter.NativeToValue(f.iterVar2Val) return f.iterVar2Val, true } @@ -1733,8 +1764,8 @@ func (f *folder) IsLocalVariable(name string) bool { // UnknownAttributePatterns implements the PartialActivation interface returning the unknown patterns // if they were provided to the input activation, or an empty set if the proxied activation is not partial. func (f *folder) UnknownAttributePatterns() []*AttributePattern { - if pv, ok := f.frame.parent.Activation.(partialActivationConverter); ok { - if partial, isPartial := pv.AsPartialActivation(); isPartial { + if f.frame.parent != nil { + if partial, isPartial := f.frame.parent.AsPartialActivation(); isPartial { return partial.UnknownAttributePatterns() } } @@ -1742,8 +1773,8 @@ func (f *folder) UnknownAttributePatterns() []*AttributePattern { } func (f *folder) AsPartialActivation() (PartialActivation, bool) { - if pv, ok := f.frame.parent.Activation.(partialActivationConverter); ok { - if _, isPartial := pv.AsPartialActivation(); isPartial { + if f.frame.parent != nil { + if _, isPartial := f.frame.parent.AsPartialActivation(); isPartial { return f, true } } diff --git a/interpreter/interpreter.go b/interpreter/interpreter.go index 07297478d..da642440d 100644 --- a/interpreter/interpreter.go +++ b/interpreter/interpreter.go @@ -50,7 +50,7 @@ type StatefulObserver interface { GetState(*ExecutionFrame) any // Observe passes the activation and relevant evaluation metadata to the observer. - // The observe method is expected to do the equivalent of GetState(AsFrame(activation)) + // The observe method is expected to do the equivalent of GetState(frame) // to find the metadata that needs to be updated upon invocation. Observe(Activation, int64, any, ref.Val) } @@ -151,7 +151,10 @@ func (et *evalStateFactory) GetState(frame *ExecutionFrame) any { // Observe records the evaluation state for a given expression node and program step. func (et *evalStateFactory) Observe(vars Activation, id int64, programStep any, val ref.Val) { - frame := AsFrame(vars) + frame, ok := vars.(*ExecutionFrame) + if !ok { + return + } if frame.ctx == nil || frame.ctx.state == nil { return } diff --git a/interpreter/interpreter_test.go b/interpreter/interpreter_test.go index 6e24a2bb6..b394121ba 100644 --- a/interpreter/interpreter_test.go +++ b/interpreter/interpreter_test.go @@ -2701,7 +2701,11 @@ func program(t testing.TB, tst *testCase, opts ...PlannerOption) (InterpretableV if err != nil { return nil, nil, err } - return prg, AsFrame(vars), nil + frame, err := NewExecutionFrame(vars) + if err != nil { + return nil, nil, err + } + return prg, frame, nil } // Check the expression. checked, errs := checker.Check(parsed, s, env) @@ -2713,7 +2717,11 @@ func program(t testing.TB, tst *testCase, opts ...PlannerOption) (InterpretableV if err != nil { return nil, nil, err } - return prg, AsFrame(vars), nil + frame, err := NewExecutionFrame(vars) + if err != nil { + return nil, nil, err + } + return prg, frame, nil } func base64Encode(val ref.Val) ref.Val { @@ -2857,15 +2865,6 @@ func funcBindings(t testing.TB, funcs ...*decls.FunctionDecl) []*functions.Overl return bindings } -type testActivationWrapper struct { - Activation - name string -} - -func (tw *testActivationWrapper) Unwrap() Activation { - return tw.Activation -} - func TestInterruptErrorIs(t *testing.T) { ie := InterruptError{} tests := []struct { @@ -3042,33 +3041,6 @@ func TestExhaustiveOperatorsLegacyEval(t *testing.T) { } } -func TestFindFrame(t *testing.T) { - frame := mustNewExecutionFrame(t, EmptyActivation()) - defer frame.Close() - - tests := []struct { - name string - act Activation - }{ - { - name: "nested wrapper", - act: &testActivationWrapper{Activation: &testActivationWrapper{Activation: frame, name: "w1"}, name: "w2"}, - }, - { - name: "parent hierarchy", - act: &parentActivationWrapper{parent: frame}, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - found := findFrame(tc.act) - if found != frame { - t.Errorf("findFrame() = %v, wanted %v", found, frame) - } - }) - } -} - func TestObservableInterpretable(t *testing.T) { obsInt := &ObservableInterpretable{InterpretableV2: NewConstValue(12, types.True)} if obsInt.ID() != 12 { @@ -3091,15 +3063,3 @@ func (t *testLegacyInterpretable) ID() int64 { func (t *testLegacyInterpretable) Eval(vars Activation) ref.Val { return types.IntOne } - -type parentActivationWrapper struct { - parent Activation -} - -func (paw *parentActivationWrapper) ResolveName(name string) (any, bool) { - return paw.parent.ResolveName(name) -} - -func (paw *parentActivationWrapper) Parent() Activation { - return paw.parent -} diff --git a/interpreter/runtimecost.go b/interpreter/runtimecost.go index 05a875af7..9266ff49b 100644 --- a/interpreter/runtimecost.go +++ b/interpreter/runtimecost.go @@ -130,5 +130,5 @@ func (c *costTrackingInterpretable) Exec(frame *ExecutionFrame) ref.Val { } func (c *costTrackingInterpretable) Eval(ctx Activation) ref.Val { - return c.Exec(AsFrame(ctx)) + return EvalActivation(ctx, c.Exec) } diff --git a/interpreter/runtimecost_test.go b/interpreter/runtimecost_test.go index 3792466c6..566c3cd0f 100644 --- a/interpreter/runtimecost_test.go +++ b/interpreter/runtimecost_test.go @@ -100,7 +100,10 @@ func TestCostObserverIntegration(t *testing.T) { if err != nil { t.Fatalf("NewActivation() failed: %v", err) } - frame := AsFrame(act) + frame, err := NewExecutionFrame(act) + if err != nil { + t.Fatalf("NewExecutionFrame() failed: %v", err) + } prg.Exec(frame) if tracker.ActualCost() != 3 { @@ -213,7 +216,10 @@ func TestListMapAccessCost(t *testing.T) { if err != nil { t.Fatalf("NewActivation() failed: %v", err) } - frame := AsFrame(act) + frame, err := NewExecutionFrame(act) + if err != nil { + t.Fatalf("NewExecutionFrame() failed: %v", err) + } res := prg.Exec(frame) actual := tracker.ActualCost() t.Logf("expr: %s => res: %v, actual cost: %d, expected: %d", expr, res, actual, expected) diff --git a/interpreter/runtimememory.go b/interpreter/runtimememory.go index 4175fb6c9..c4d8a02ac 100644 --- a/interpreter/runtimememory.go +++ b/interpreter/runtimememory.go @@ -86,8 +86,8 @@ func (mt *memoryTrackerFactory) GetState(frame *ExecutionFrame) any { // peak at the expression nodes which produced them; constants are part of the program image // rather than runtime-materialized memory and are not observed. func (mt *memoryTrackerFactory) Observe(vars Activation, id int64, programStep any, val ref.Val) { - frame := AsFrame(vars) - if frame == nil || frame.ctx == nil || frame.ctx.memory == nil { + frame, ok := vars.(*ExecutionFrame) + if !ok || frame.ctx == nil || frame.ctx.memory == nil { return } tracker := frame.ctx.memory diff --git a/interpreter/runtimememory_test.go b/interpreter/runtimememory_test.go index cf3d4b028..4a247c49e 100644 --- a/interpreter/runtimememory_test.go +++ b/interpreter/runtimememory_test.go @@ -371,7 +371,10 @@ func benchmarkMemoryTracker(b *testing.B, expr string, vars []*decls.VariableDec b.Fatalf("NewInterpretable() failed: %v", err) } - frame := AsFrame(constructTestActivation(b, in)) + frame, err := NewExecutionFrame(constructTestActivation(b, in)) + if err != nil { + b.Fatalf("NewExecutionFrame() failed: %v", err) + } b.ResetTimer() b.ReportAllocs() @@ -494,7 +497,10 @@ func runOptionsBenchmark(b *testing.B, expr string, vars []*decls.VariableDecl, b.Fatalf("NewInterpretable() failed: %v", err) } - frame := AsFrame(constructTestActivation(b, in)) + frame, err := NewExecutionFrame(constructTestActivation(b, in)) + if err != nil { + b.Fatalf("NewExecutionFrame() failed: %v", err) + } b.ResetTimer() b.ReportAllocs()