Skip to content

Optimize vm a bit by reducing unnecessary calls and allocating includ… #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ajroetker merged 3 commits into from
Jun 25, 2025
Merged

Optimize vm a bit by reducing unnecessary calls and allocating includ… #47

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4a39e1
Optimize vm a bit by reducing unnecessary calls and allocating includ…
Jun 24, 2025
dd5399e
Fix issues
Jun 24, 2025
02a7506
Fix issues
Jun 24, 2025
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
93 changes: 38 additions & 55 deletions vm/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ import (
u "github.com/araddon/gou"
"github.com/mb0/glob"

"slices"

"github.com/lytics/qlbridge/expr"
"github.com/lytics/qlbridge/lex"
"github.com/lytics/qlbridge/value"
"slices"
)

var (
Expand All @@ -42,7 +43,8 @@ type EvalBaseContext struct {
// evaluated against. It may be a simple reader of message/data or any
// object whhich implements EvalContext.
func Eval(ctx expr.EvalContext, arg expr.Node) (value.Value, bool) {
return evalDepth(ctx, arg, 0, make([]string, 0))
// Initialize a visited includes stack of 10
return evalDepth(ctx, arg, 0, make([]string, 0, 10))
}

// creates a new Value with a nil group and given value.
Expand All @@ -52,12 +54,10 @@ func numberNodeToValue(t *expr.NumberNode) (value.Value, bool) {
} else if t.IsFloat {
fv, ok := value.StringToFloat64(t.Text)
if !ok {
u.Debugf("Could not perform numeric conversion for %q", t.Text)
return value.NilValueVal, false
}
return value.NewNumberValue(fv), true
}
u.Debugf("Could not find numeric conversion for %#v", t)
return value.NilValueVal, false
}

Expand All @@ -66,7 +66,7 @@ func numberNodeToValue(t *expr.NumberNode) (value.Value, bool) {
// InlineIncludes alternative in expr pkg which actually re-writes the expression
// to remove includes and embed the expressions they refer to as part of this expression.
func ResolveIncludes(ctx expr.Includer, arg expr.Node) error {
return resolveIncludesDepth(ctx, arg, 0, make([]string, 0))
return resolveIncludesDepth(ctx, arg, 0, make([]string, 0, 10))
}
func resolveIncludesDepth(ctx expr.Includer, arg expr.Node, depth int, visitedIncludes []string) error {
if depth > MaxDepth {
Expand Down Expand Up @@ -137,17 +137,17 @@ func evalDepth(ctx expr.EvalContext, arg expr.Node, depth int, visitedIncludes [
case *expr.NumberNode:
return numberNodeToValue(argVal)
case *expr.BinaryNode:
return walkBinary(ctx, argVal, depth, visitedIncludes)
return evalBinary(ctx, argVal, depth, visitedIncludes)
case *expr.BooleanNode:
return walkBoolean(ctx, argVal, depth, visitedIncludes)
case *expr.UnaryNode:
return walkUnary(ctx, argVal)
return walkUnary(ctx, argVal, depth, visitedIncludes)
case *expr.TriNode:
return walkTernary(ctx, argVal)
return walkTernary(ctx, argVal, depth, visitedIncludes)
case *expr.ArrayNode:
return walkArray(ctx, argVal)
return walkArray(ctx, argVal, depth, visitedIncludes)
case *expr.FuncNode:
return walkFunc(ctx, argVal)
return walkFunc(ctx, argVal, depth, visitedIncludes)
case *expr.IdentityNode:
return walkIdentity(ctx, argVal)
case *expr.StringNode:
Expand Down Expand Up @@ -196,11 +196,9 @@ func resolveInclude(ctx expr.Includer, inc *expr.IncludeNode, depth int, visited
if err == expr.ErrNoIncluder {
return err
}
u.Debugf("Could not find include for filter:%s err=%v", inc.String(), err)
return err
}
if incExpr == nil {
u.Debugf("Includer %T returned a nil filter statement!", inc)
return expr.ErrIncludeNotFound
}
if err = resolveIncludesDepth(ctx, incExpr, depth+1, visitedIncludes); err != nil {
Expand Down Expand Up @@ -355,13 +353,6 @@ func walkBoolean(ctx expr.EvalContext, n *expr.BooleanNode, depth int, visitedIn
// x OR y
// x > y
// x < =
func walkBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedIncludes []string) (value.Value, bool) {
Copy link
Contributor

@janveltm janveltm Jun 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't really know much about the logic around the ok variable. But maybe to be safe change the returned value to nil wherever ok==false in evalBinary?

Copy link
Contributor Author

@ajroetker ajroetker Jun 24, 2025

Choose a reason for hiding this comment

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

done!

val, ok := evalBinary(ctx, node, depth, visitedIncludes)
if !ok {
return nil, ok
}
return val, ok
}
func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedIncludes []string) (value.Value, bool) {
ar, aok := evalDepth(ctx, node.Args[0], depth+1, visitedIncludes)
br, bok := evalDepth(ctx, node.Args[1], depth+1, visitedIncludes)
Expand Down Expand Up @@ -444,7 +435,6 @@ func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedI
}
return value.NewBoolValue(false), true
default:
u.Debugf("unsupported op for SliceValue op:%v rhT:%T", node.Operator, br)
return nil, false
}
case nil, value.NilValue:
Expand Down Expand Up @@ -477,7 +467,6 @@ func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedI
return value.BoolValueTrue, true
}
default:
u.Debugf("Could not coerce to number: T:%T v:%v", val, val)
}
}
return value.BoolValueFalse, true
Expand Down Expand Up @@ -548,7 +537,6 @@ func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedI
}
return value.NewBoolValue(true), true
default:
u.Debugf("unsupported op: %v", node.Operator)
return nil, false
}
case value.Slice:
Expand Down Expand Up @@ -576,7 +564,6 @@ func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedI
}
return value.NewBoolValue(false), true
default:
u.Debugf("unsupported op for SliceValue op:%v rhT:%T", node.Operator, br)
return nil, false
}
case value.BoolValue:
Expand All @@ -587,7 +574,6 @@ func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedI
case lex.TokenNE:
return value.NewBoolValue(value.BoolStringVal(at.Val()) != bt.Val()), true
default:
u.Debugf("unsupported op: %v", node.Operator)
}
}
switch node.Operator.T {
Expand All @@ -596,7 +582,6 @@ func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedI
return value.NewBoolValue(false), true
}
// Should we evaluate strings that are non-nil to be = true?
u.Debugf("not handled: boolean %v %T=%v expr: %s", node.Operator, at.Value(), at.Val(), node.String())
return nil, false
case value.Map:
switch node.Operator.T {
Expand All @@ -607,7 +592,6 @@ func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedI
}
return value.NewBoolValue(false), true
default:
u.Debugf("unsupported op for Map op:%v rhT:%T", node.Operator, br)
return nil, false
}

Expand All @@ -620,7 +604,7 @@ func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedI
case value.TimeValue:
lht, ok := value.ValueToTime(at)
if !ok {
return value.BoolValueFalse, false
return nil, false
}
return operateTime(node.Operator.T, lht, bt.Val())
default:
Expand Down Expand Up @@ -780,22 +764,23 @@ func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedI
lht := at.Val()
rht, ok := value.ValueToTime(br)
if !ok {
return value.BoolValueFalse, false
return nil, false
}

return operateTime(node.Operator.T, lht, rht)

case value.Map:
rhvals := make([]string, 0)
var rhvals []string
switch bv := br.(type) {
case value.StringsValue:
rhvals = bv.Val()
case value.Slice:
for _, arg := range bv.SliceValue() {
rhvals = append(rhvals, arg.ToString())
sliceValue := bv.SliceValue()
rhvals = make([]string, len(sliceValue))
for i, arg := range sliceValue {
rhvals[i] = arg.ToString()
}
default:
u.Debugf("un-handled? %T", bv)
return nil, false
}

Expand Down Expand Up @@ -831,16 +816,17 @@ func evalBinary(ctx expr.EvalContext, node *expr.BinaryNode, depth int, visitedI
case lex.TokenNE:
return value.NewBoolValue(true), true
case lex.TokenContains, lex.TokenLike, lex.TokenIN, lex.TokenIntersects:
return value.NewBoolValue(false), false
return nil, false
default:
return nil, false
}
default:
u.Debugf("Unknown op? %T %T %v", ar, at, ar)
return value.NewErrorValue(fmt.Errorf("unsupported left side value: %T in %s", at, node)), false
// return value.NewErrorValue(fmt.Errorf("unsupported left side value: %T in %s", at, node)), false
return nil, false
}

return value.NewErrorValue(fmt.Errorf("unsupported binary expression: %s", node)), false
// return value.NewErrorValue(fmt.Errorf("unsupported binary expression: %s", node)), false
return nil, false
}

func walkIdentity(ctx expr.EvalContext, node *expr.IdentityNode) (value.Value, bool) {
Expand All @@ -857,17 +843,16 @@ func walkIdentity(ctx expr.EvalContext, node *expr.IdentityNode) (value.Value, b
return ctx.Get(node.Text)
}

func walkUnary(ctx expr.EvalContext, node *expr.UnaryNode) (value.Value, bool) {
func walkUnary(ctx expr.EvalContext, node *expr.UnaryNode, depth int, visitedIncludes []string) (value.Value, bool) {

a, ok := Eval(ctx, node.Arg)
a, ok := evalDepth(ctx, node.Arg, depth, visitedIncludes)
if !ok {
switch node.Operator.T {
case lex.TokenExists:
return value.NewBoolValue(false), true
case lex.TokenNegate:
return value.NewBoolValue(false), false
}
u.Debugf("unary could not evaluate for[ %s ] and %#v", node.String(), node)
return a, false
}

Expand All @@ -879,7 +864,7 @@ func walkUnary(ctx expr.EvalContext, node *expr.UnaryNode) (value.Value, bool) {
case nil, value.NilValue:
return value.NewBoolValue(false), false
default:
u.LogThrottle(u.WARN, 5, "unary type not implemented. Unknonwn node type: %T:%v node=%s", argVal, argVal, node.String())
// u.LogThrottle(u.WARN, 5, "unary type not implemented. Unknonwn node type: %T:%v node=%s", argVal, argVal, node.String())
return value.NewNilValue(), false
}
case lex.TokenMinus:
Expand All @@ -905,18 +890,18 @@ func walkUnary(ctx expr.EvalContext, node *expr.UnaryNode) (value.Value, bool) {
// walkTernary ternary evaluator
//
// A BETWEEN B AND C
func walkTernary(ctx expr.EvalContext, node *expr.TriNode) (value.Value, bool) {
func walkTernary(ctx expr.EvalContext, node *expr.TriNode, depth int, visitedIncludes []string) (value.Value, bool) {

a, aok := Eval(ctx, node.Args[0])
b, bok := Eval(ctx, node.Args[1])
c, cok := Eval(ctx, node.Args[2])
if !aok {
a, aok := evalDepth(ctx, node.Args[0], depth, visitedIncludes)
if a == nil || !aok {
return nil, false
}
if !bok || !cok {
b, bok := evalDepth(ctx, node.Args[1], depth, visitedIncludes)
if b == nil || !bok {
return nil, false
}
if a == nil || b == nil || c == nil {
c, cok := evalDepth(ctx, node.Args[2], depth, visitedIncludes)
if c == nil || !cok {
return nil, false
}
switch node.Operator.T {
Expand Down Expand Up @@ -944,7 +929,6 @@ func walkTernary(ctx expr.EvalContext, node *expr.TriNode) (value.Value, bool) {
}
return value.NewBoolValue(false), true
case value.NumberValue:

av := at.Val()
bv, ok := value.ValueToFloat64(b)
if !ok {
Expand All @@ -966,7 +950,6 @@ func walkTernary(ctx expr.EvalContext, node *expr.TriNode) (value.Value, bool) {
return value.NewBoolValue(false), true

case value.TimeValue:

av := at.Val()
bv, ok := value.ValueToTime(b)
if !ok {
Expand Down Expand Up @@ -1000,12 +983,12 @@ func walkTernary(ctx expr.EvalContext, node *expr.TriNode) (value.Value, bool) {
// walkArray Array evaluator: evaluate multiple values into an array
//
// (b,c,d)
func walkArray(ctx expr.EvalContext, node *expr.ArrayNode) (value.Value, bool) {
func walkArray(ctx expr.EvalContext, node *expr.ArrayNode, depth int, visitedIncludes []string) (value.Value, bool) {

vals := make([]value.Value, len(node.Args))

for i := range node.Args {
v, _ := Eval(ctx, node.Args[i])
v, _ := evalDepth(ctx, node.Args[i], depth, visitedIncludes)
vals[i] = v
}

Expand All @@ -1014,20 +997,20 @@ func walkArray(ctx expr.EvalContext, node *expr.ArrayNode) (value.Value, bool) {
}

// walkFunc evaluates a function
func walkFunc(ctx expr.EvalContext, node *expr.FuncNode) (value.Value, bool) {
func walkFunc(ctx expr.EvalContext, node *expr.FuncNode, depth int, visitedIncludes []string) (value.Value, bool) {

if node.F.CustomFunc == nil {
return nil, false
}
if node.Eval == nil {
u.LogThrottle(u.WARN, 10, "No Eval() for %s", node.Name)
// u.LogThrottle(u.WARN, 10, "No Eval() for %s", node.Name)
return nil, false
}

args := make([]value.Value, len(node.Args))

for i, a := range node.Args {
v, ok := Eval(ctx, a)
v, ok := evalDepth(ctx, a, depth, visitedIncludes)
if !ok {
v = value.NewNilValue()
}
Expand Down Expand Up @@ -1186,14 +1169,14 @@ func operateTime(op lex.TokenType, lht, rht time.Time) (value.BoolValue, bool) {
}
return value.BoolValueFalse, true
default:
u.Debugf("unhandled date op %v", op)
}
return value.BoolValueFalse, false
}

// LikeCompare takes two strings and evaluates them for like equality
func LikeCompare(a, b string) (value.BoolValue, bool) {
// Do we want to always do this replacement? Or do this at parse time or config?
//
b = strings.Replace(b, "%", "*", -1)
match, err := glob.Match(b, a)
if err != nil {
Expand Down