Skip to content

Commit

Permalink
Revert "Evaluation results are now cached and reused between rules."
Browse files Browse the repository at this point in the history
This reverts commit 68cb190.
  • Loading branch information
Pascal-Delange committed Feb 6, 2025
1 parent 17ea282 commit 7d43fec
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 272 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ require (
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/mfridman/interpolate v0.0.2 // indirect
github.com/mitchellh/go-wordwrap v1.0.1 // indirect
github.com/mitchellh/hashstructure/v2 v2.0.2 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
github.com/moby/docker-image-spec v1.3.1 // indirect
github.com/moby/term v0.5.0 // indirect
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,6 @@ github.com/mfridman/interpolate v0.0.2 h1:pnuTK7MQIxxFz1Gr+rjSIx9u7qVjf5VOoM/u6B
github.com/mfridman/interpolate v0.0.2/go.mod h1:p+7uk6oE07mpE/Ik1b8EckO0O4ZXiGAfshKBWLUM9Xg=
github.com/mitchellh/go-wordwrap v1.0.1 h1:TLuKupo69TCn6TQSyGxwI1EblZZEsQ0vMlAFQflz0v0=
github.com/mitchellh/go-wordwrap v1.0.1/go.mod h1:R62XHJLzvMFRBbcrT7m7WgmE1eOyTSsCt+hzestvNj0=
github.com/mitchellh/hashstructure/v2 v2.0.2 h1:vGKWl0YJqUNxE8d+h8f6NJLcCJrgbhC4NcD46KavDd4=
github.com/mitchellh/hashstructure/v2 v2.0.2/go.mod h1:MG3aRVU/N29oo/V/IhBX8GR/zz4kQkprJgF2EVszyDE=
github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY=
github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0=
Expand Down
9 changes: 1 addition & 8 deletions models/ast/ast_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ import (
"fmt"

"github.com/cockroachdb/errors"
"github.com/mitchellh/hashstructure/v2"
)

type Node struct {
Index int `hash:"ignore"`
Index int

// A node is a constant xOR a function
Function Function
Expand Down Expand Up @@ -53,12 +52,6 @@ func (node Node) ReadConstantNamedChildString(name string) (string, error) {
return value, nil
}

func (node Node) Hash() uint64 {
hash, _ := hashstructure.Hash(node, hashstructure.FormatV2, nil)

return hash
}

// Cost calculates the weights of an AST subtree to reorder, when the parent is commutative,
// nodes to prioritize faster ones.
func (node Node) Cost() int {
Expand Down
128 changes: 38 additions & 90 deletions usecases/ast_eval/evaluate_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,12 @@ package ast_eval

import (
"context"
"fmt"
"sync"

"github.com/checkmarble/marble-backend/models/ast"
"github.com/checkmarble/marble-backend/pure_utils"
"golang.org/x/sync/singleflight"
)

type EvaluationCache struct {
Cache sync.Map
Executor *singleflight.Group
}

func NewEvaluationCache() *EvaluationCache {
return &EvaluationCache{
Cache: sync.Map{},
Executor: new(singleflight.Group),
}
}

func EvaluateAst(ctx context.Context, cache *EvaluationCache,
environment AstEvaluationEnvironment, node ast.Node,
) (ast.NodeEvaluation, bool) {
func EvaluateAst(ctx context.Context, environment AstEvaluationEnvironment, node ast.Node) (ast.NodeEvaluation, bool) {
// Early exit for constant, because it should have no children.
if node.Function == ast.FUNC_CONSTANT {
return ast.NodeEvaluation{
Expand All @@ -35,29 +18,13 @@ func EvaluateAst(ctx context.Context, cache *EvaluationCache,
}, true
}

type nodeEvaluationResponse struct {
eval ast.NodeEvaluation
ok bool
}

hash := node.Hash()

if cache != nil {
if cached, ok := cache.Cache.Load(hash); ok {
response := cached.(nodeEvaluationResponse)
response.eval.Index = node.Index

return response.eval, response.ok
}
}

childEvaluationFail := false

// Only interested in lazy callback which will have default value if an error is returned
attrs, _ := node.Function.Attributes()

evalChild := func(child ast.Node) (childEval ast.NodeEvaluation, evalNext bool) {
childEval, ok := EvaluateAst(ctx, cache, environment, child)
childEval, ok := EvaluateAst(ctx, environment, child)

if !ok {
childEvaluationFail = true
Expand All @@ -71,72 +38,53 @@ func EvaluateAst(ctx context.Context, cache *EvaluationCache,
return
}

cachedExecutor := new(singleflight.Group)
weightedNodes := NewWeightedNodes(environment, node, node.Children)

if cache != nil {
cachedExecutor = cache.Executor
// eval each child
evaluation := ast.NodeEvaluation{
Index: node.Index,
Function: node.Function,
Children: weightedNodes.Reorder(pure_utils.MapWhile(weightedNodes.Sorted(), evalChild)),
NamedChildren: pure_utils.MapValuesWhile(node.NamedChildren, evalChild),
}

eval, _, _ := cachedExecutor.Do(fmt.Sprintf("%d", hash), func() (any, error) {
weightedNodes := NewWeightedNodes(environment, node, node.Children)

// eval each child
evaluation := ast.NodeEvaluation{
Index: node.Index,
Function: node.Function,
Children: weightedNodes.Reorder(pure_utils.MapWhile(weightedNodes.Sorted(), evalChild)),
NamedChildren: pure_utils.MapValuesWhile(node.NamedChildren, evalChild),
}

if childEvaluationFail {
// an error occurred in at least one of the children. Stop the evaluation.
if childEvaluationFail {
// an error occurred in at least one of the children. Stop the evaluation.

// the frontend expects an ErrUndefinedFunction error to be present even when no evaluation happened.
if node.Function == ast.FUNC_UNDEFINED {
evaluation.Errors = append(evaluation.Errors, ast.ErrUndefinedFunction)
}

return nodeEvaluationResponse{evaluation, false}, nil
}

getReturnValue := func(e ast.NodeEvaluation) any { return e.ReturnValue }
arguments := ast.Arguments{
Args: pure_utils.Map(evaluation.Children, getReturnValue),
NamedArgs: pure_utils.MapValues(evaluation.NamedChildren, getReturnValue),
}

evaluator, err := environment.GetEvaluator(node.Function)
if err != nil {
evaluation.Errors = append(evaluation.Errors, err)
return nodeEvaluationResponse{evaluation, false}, nil
// the frontend expects an ErrUndefinedFunction error to be present even when no evaluation happened.
if node.Function == ast.FUNC_UNDEFINED {
evaluation.Errors = append(evaluation.Errors, ast.ErrUndefinedFunction)
}

evaluation.ReturnValue, evaluation.Errors = evaluator.Evaluate(ctx, arguments)

if evaluation.Errors == nil {
// Assign an empty array to indicate that the evaluation occured.
// The evaluator is not supposed to return a nil array of errors, but let's be nice.
evaluation.Errors = []error{}
}
return evaluation, false
}

ok := len(evaluation.Errors) == 0
getReturnValue := func(e ast.NodeEvaluation) any { return e.ReturnValue }
arguments := ast.Arguments{
Args: pure_utils.Map(evaluation.Children, getReturnValue),
NamedArgs: pure_utils.MapValues(evaluation.NamedChildren, getReturnValue),
}

if !ok {
// The evaluator is supposed to return nil ReturnValue when an error is present.
evaluation.ReturnValue = nil
}
evaluator, err := environment.GetEvaluator(node.Function)
if err != nil {
evaluation.Errors = append(evaluation.Errors, err)
return evaluation, false
}

evaluationResponse := nodeEvaluationResponse{evaluation, ok}
evaluation.ReturnValue, evaluation.Errors = evaluator.Evaluate(ctx, arguments)

if cache != nil {
cache.Cache.Store(hash, evaluationResponse)
}
if evaluation.Errors == nil {
// Assign an empty array to indicate that the evaluation occured.
// The evaluator is not supposed to return a nil array of errors, but let's be nice.
evaluation.Errors = []error{}
}

return evaluationResponse, nil
})
ok := len(evaluation.Errors) == 0

evaluation := eval.(nodeEvaluationResponse)
evaluation.eval.Index = node.Index
if !ok {
// The evaluator is supposed to return nil ReturnValue when an error is present.
evaluation.ReturnValue = nil
}

return evaluation.eval, evaluation.ok
return evaluation, ok
}
3 changes: 1 addition & 2 deletions usecases/ast_eval/evaluate_ast_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ type EvaluateAstExpression struct {

func (evaluator *EvaluateAstExpression) EvaluateAstExpression(
ctx context.Context,
cache *EvaluationCache,
ruleAstExpression ast.Node,
organizationId string,
payload models.ClientObject,
Expand All @@ -28,7 +27,7 @@ func (evaluator *EvaluateAstExpression) EvaluateAstExpression(
DatabaseAccessReturnFakeValue: false,
})

evaluation, ok := EvaluateAst(ctx, cache, environment, ruleAstExpression)
evaluation, ok := EvaluateAst(ctx, environment, ruleAstExpression)
if !ok {
return evaluation, errors.Join(evaluation.FlattenErrors()...)
}
Expand Down
Loading

0 comments on commit 7d43fec

Please sign in to comment.