Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 36 additions & 9 deletions Sources/Rego/IREvaluator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ internal struct IREvaluationContext {
var policy: IndexedIRPolicy
var maxCallDepth: Int = 16_384
var callDepth: Int = 0
var callMemo = MemoStack() // shared stack of memoCache structs

init(ctx: EvaluationContext, policy: IndexedIRPolicy) {
self.ctx = ctx
self.policy = policy
}

func withIncrementedCallDepth() -> IREvaluationContext {
var ctx = self
Expand Down Expand Up @@ -911,13 +917,16 @@ func evalBlock(
var newLocals = currentScopePtr.v.locals
newLocals[stmt.local] = patched
currentScopePtr = framePtr.v.pushScope(locals: newLocals)
// Start executing the block on the new scope we just pushed
let blockResult = try await evalBlock(
withContext: ctx,
framePtr: framePtr,
caller: statement,
block: stmt.block
)

let blockResult = try await ctx.callMemo.withPush {
Copy link
Contributor

@philippzagar philippzagar Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet, I like the with-style syntax in this use case, great idea that prohibits forgetting to pop.

// Start executing the block on the new scope we just pushed
return try await evalBlock(
withContext: ctx,
framePtr: framePtr,
caller: statement,
block: stmt.block
)
}

// Squash locals from the child frame back into the current frame.
// Overlay the original (non-patched) value, keep the other side-effects.
Expand Down Expand Up @@ -957,6 +966,13 @@ private func evalCall(
args: [IR.Operand],
isDynamic: Bool = false
) async throws -> AST.RegoValue {
// Check memo cache if applicable to save repeated evaluation time for rules
let shouldMemoize = args.count == 2 // Currently support _rules_, not _functions_
let sig = InvocationKey(funcName: funcName, args: args)
if shouldMemoize, let cachedResult = ctx.callMemo[sig] {
return cachedResult
}

var argValues: [AST.RegoValue] = []
for arg in args {
// Note: we do not enforce that args are defined here, it appears
Expand All @@ -974,24 +990,35 @@ private func evalCall(
return .undefined
}

return try await callPlanFunc(
let result = try await callPlanFunc(
ctx: ctx,
frame: frame,
caller: caller,
funcName: funcName,
args: argValues
)

if shouldMemoize {
ctx.callMemo[sig] = result
}
return result
}

// Handle plan-defined functions first
if ctx.policy.funcs[funcName] != nil {
return try await callPlanFunc(
let result = try await callPlanFunc(
ctx: ctx,
frame: frame,
caller: caller,
funcName: funcName,
args: argValues
)

if shouldMemoize {
ctx.callMemo[sig] = result
}

return result
}

// Handle built-in functions last
Expand Down
60 changes: 60 additions & 0 deletions Sources/Rego/Memo.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import AST
import Foundation
import IR

/// InvocationKey is a key for memoizing an IR function call invocation.
/// Note we capture the arguments as unresolved operands and not resolved values,
/// as hashing the values was proving extremely expensive. We instead rely on the
/// invariant the the plan / evaluator will not modify a local after it has been initally set.
struct InvocationKey: Hashable {
let funcName: String
let args: [IR.Operand]
}

/// MemoCache is a memoization cache of plan invocations
typealias MemoCache = [InvocationKey: AST.RegoValue]

/// MemoStack is a stack of MemoCaches
final class MemoStack {
var stack: [MemoCache] = []
}

extension MemoStack {
/// Get and set values on the cache at the top of the memo stack.
subscript(key: InvocationKey) -> AST.RegoValue? {
get {
guard !self.stack.isEmpty else {
return nil
}
return self.stack[self.stack.count - 1][key]
}
set {
if self.stack.isEmpty {
self.stack.append(MemoCache.init())
}
self.stack[self.stack.count - 1][key] = newValue
}
}

func push() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Mark these funcs as @inlineable, should enable the compiler to inline these calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I suggest we revisit this optimization if we see any measurable impact - my hunch is that we will not see excessive with statements in the hot path to the extent that a function call will be an issue.

self.stack.append(MemoCache.init())
}

func pop() {
guard !self.stack.isEmpty else {
return
}
self.stack.removeLast()
}

/// withPush returns the result of calling the provided closure with
/// a fresh memoCache pushed on the stack. The memoCache will only be
/// active during that call, and discarded when it completes.
func withPush<T>(_ body: () async throws -> T) async rethrows -> T {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use sending T here, but it doesn't make a big difference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, probably a good idea to explicitly pass down the isolation domain via:
isolation: isolated (any Actor)? = #isolation,
so that we avoid an actor hop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me that those are needed here? We're not doing an actor hop that I can see, and this stack is only used within a single evaluation, not across evaluations. Please let me know if I'm misunderstanding 🤔

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any method without isolation inheritance is going to hop defensively. The recommendation with Swift 6.2 is to adopt the upcoming language feature NonIsolatedNonSendingByDefault which makes all async methods nonisolated(nonsending)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shomron That shouldn't be a huge performance loss in this case, however this applies to the entirety of swift-opa and should be kept in mind

self.push()
defer {
self.pop()
}
return try await body()
}
}