Skip to content

Commit

Permalink
planner: don't plan superfluous Equal/NotEqualStmts (#6386)
Browse files Browse the repository at this point in the history
Basically lifting this compiler optiimization for the Wasm compiler into the planning stage: If we already know at plan time that a certain (in)equality check fails/succeeds, we don't need to do it. The less work, the better.

* planner: don't emit `NotEqualStmt{A: ..., B: false}` where superfluous
* planner: don't emit `EqualStmt{A: x, B: x}`
   for string and bool constants
* compiler/wasm: remove optimizations

Signed-off-by: Stephan Renatus <[email protected]>
  • Loading branch information
srenatus authored Nov 6, 2023
1 parent 11f2d7f commit 2acef3b
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 26 deletions.
24 changes: 4 additions & 20 deletions internal/compiler/wasm/wasm.go
Original file line number Diff line number Diff line change
Expand Up @@ -1085,27 +1085,11 @@ func (c *Compiler) compileBlock(block *ir.Block) ([]instruction.Instruction, err
instrs = append(instrs, instruction.Call{Index: c.function(opaNumberSize)})
instrs = append(instrs, instruction.SetLocal{Index: c.local(stmt.Target)})
case *ir.EqualStmt:
if stmt.A != stmt.B { // constants, or locals, being equal here can skip the check
instrs = append(instrs, c.instrRead(stmt.A))
instrs = append(instrs, c.instrRead(stmt.B))
instrs = append(instrs, instruction.Call{Index: c.function(opaValueCompare)})
instrs = append(instrs, instruction.BrIf{Index: 0})
}
instrs = append(instrs, c.instrRead(stmt.A))
instrs = append(instrs, c.instrRead(stmt.B))
instrs = append(instrs, instruction.Call{Index: c.function(opaValueCompare)})
instrs = append(instrs, instruction.BrIf{Index: 0})
case *ir.NotEqualStmt:
if stmt.A == stmt.B { // same local, same bool constant, or same string constant
instrs = append(instrs, instruction.Br{Index: 0})
continue
}
_, okA := stmt.A.Value.(ir.Bool)
if _, okB := stmt.B.Value.(ir.Bool); okA && okB {
// not equal (checked above), but both booleans => not equal
continue
}
_, okA = stmt.A.Value.(ir.StringIndex)
if _, okB := stmt.B.Value.(ir.StringIndex); okA && okB {
// not equal (checked above), but both strings => not equal
continue
}
instrs = append(instrs, c.instrRead(stmt.A))
instrs = append(instrs, c.instrRead(stmt.B))
instrs = append(instrs, instruction.Call{Index: c.function(opaValueCompare)})
Expand Down
46 changes: 40 additions & 6 deletions internal/planner/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -841,13 +841,29 @@ func (p *Planner) dataRefsShadowRuletrie(refs []ast.Ref) bool {
}

func (p *Planner) planExprTerm(e *ast.Expr, iter planiter) error {
return p.planTerm(e.Terms.(*ast.Term), func() error {
p.appendStmt(&ir.NotEqualStmt{
A: p.ltarget,
B: op(ir.Bool(false)),
// NOTE(sr): There are only three cases to deal with when we see a naked term
// in a rule body:
// 1. it's `false` -- so we can stop, emit a break stmt
// 2. it's a var or a ref, like `input` or `data.foo.bar`, where we need to
// check what it ends up being (at run time) to determine if it's not false
// 3. it's any other term -- `true`, a string, a number, whatever. We can skip
// that, since it's true-ish enough for evaluating the rule body.
switch t := e.Terms.(*ast.Term).Value.(type) {
case ast.Boolean:
if !bool(t) { // We know this cannot hold, break unconditionally
p.appendStmt(&ir.BreakStmt{})
return iter()
}
case ast.Ref, ast.Var: // We don't know these at plan-time
return p.planTerm(e.Terms.(*ast.Term), func() error {
p.appendStmt(&ir.NotEqualStmt{
A: p.ltarget,
B: op(ir.Bool(false)),
})
return iter()
})
return iter()
})
}
return iter()
}

func (p *Planner) planExprEvery(e *ast.Expr, iter planiter) error {
Expand Down Expand Up @@ -1203,6 +1219,24 @@ func (p *Planner) planUnifyVar(a ast.Var, b *ast.Term, iter planiter) error {
}

func (p *Planner) planUnifyLocal(a ir.Operand, b *ast.Term, iter planiter) error {
// special cases: when a is StringIndex or Bool, and b is a string, or a bool, we can shortcut
switch va := a.Value.(type) {
case ir.StringIndex:
if vb, ok := b.Value.(ast.String); ok {
if va != ir.StringIndex(p.getStringConst(string(vb))) {
p.appendStmt(&ir.BreakStmt{})
}
return iter() // Don't plan EqualStmt{A: "foo", B: "foo"}
}
case ir.Bool:
if vb, ok := b.Value.(ast.Boolean); ok {
if va != ir.Bool(vb) {
p.appendStmt(&ir.BreakStmt{})
}
return iter() // Don't plan EqualStmt{A: true, B: true}
}
}

switch vb := b.Value.(type) {
case ast.Null, ast.Boolean, ast.Number, ast.String, ast.Ref, ast.Set, *ast.SetComprehension, *ast.ArrayComprehension, *ast.ObjectComprehension:
return p.planTerm(b, func() error {
Expand Down

0 comments on commit 2acef3b

Please sign in to comment.