Skip to content

Commit

Permalink
Implement operators == and != as builtins (google#221)
Browse files Browse the repository at this point in the history
People use these operators in tight loops, without even
thinking about it, and it's previous implementation required
multiple object lookups (std.), string comparisons (for types)
and multiple jsonnet function calls.

This change introduces builtin, efficient implementation.
It results in ~3x speedup in strContains benchmark that
Angus provided on Slack.

Additional benefit is that equals/primitiveEquals distinction
is now obsolete, which made things simpler for everyone.
  • Loading branch information
sbarzowski authored and sparkprime committed May 24, 2018
1 parent d9b833b commit 530cf7b
Show file tree
Hide file tree
Showing 14 changed files with 148 additions and 40 deletions.
140 changes: 133 additions & 7 deletions builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,8 @@ func builtinUnaryMinus(e *evaluator, xp potentialValue) (value, error) {
return makeValueNumber(-x.value), nil
}

// TODO(sbarzowski) since we have a builtin implementation of equals it's no longer really
// needed and we should deprecate it eventually
func primitiveEquals(e *evaluator, xp potentialValue, yp potentialValue) (value, error) {
x, err := e.evaluate(xp)
if err != nil {
Expand Down Expand Up @@ -492,6 +494,131 @@ func primitiveEquals(e *evaluator, xp potentialValue, yp potentialValue) (value,
}
}

func rawEquals(e *evaluator, x, y value) (bool, error) {
if x.getType() != y.getType() {
return false, nil
}
switch left := x.(type) {
case *valueBoolean:
right, err := e.getBoolean(y)
if err != nil {
return false, err
}
return left.value == right.value, nil
case *valueNumber:
right, err := e.getNumber(y)
if err != nil {
return false, err
}
return left.value == right.value, nil
case *valueString:
right, err := e.getString(y)
if err != nil {
return false, err
}
return stringEqual(left, right), nil
case *valueNull:
return true, nil
case *valueArray:
right, err := e.getArray(y)
if err != nil {
return false, err
}
if left.length() != right.length() {
return false, nil
}
for i := range left.elements {
leftElem, err := e.evaluate(left.elements[i])
if err != nil {
return false, err
}
rightElem, err := e.evaluate(right.elements[i])
if err != nil {
return false, err
}
eq, err := rawEquals(e, leftElem, rightElem)
if err != nil {
return false, err
}
if !eq {
return false, nil
}
}
return true, nil
case valueObject:
right, err := e.getObject(y)
if err != nil {
return false, err
}
leftFields := objectFields(left, withoutHidden)
rightFields := objectFields(right, withoutHidden)
sort.Strings(leftFields)
sort.Strings(rightFields)
if len(leftFields) != len(rightFields) {
return false, nil
}
for i := range leftFields {
if leftFields[i] != rightFields[i] {
return false, nil
}
}
for i := range leftFields {
fieldName := leftFields[i]
leftField, err := left.index(e, fieldName)
if err != nil {
return false, err
}
rightField, err := right.index(e, fieldName)
if err != nil {
return false, err
}
eq, err := rawEquals(e, leftField, rightField)
if err != nil {
return false, err
}
if !eq {
return false, nil
}
}
return true, nil
case *valueFunction:
return false, e.Error("Cannot test equality of functions")
}
panic(fmt.Sprintf("Unhandled case in equals %#+v %#+v", x, y))
}

func builtinEquals(e *evaluator, xp potentialValue, yp potentialValue) (value, error) {
x, err := e.evaluate(xp)
if err != nil {
return nil, err
}
y, err := e.evaluate(yp)
if err != nil {
return nil, err
}
eq, err := rawEquals(e, x, y)
if err != nil {
return nil, err
}
return makeValueBoolean(eq), nil
}

func builtinNotEquals(e *evaluator, xp potentialValue, yp potentialValue) (value, error) {
x, err := e.evaluate(xp)
if err != nil {
return nil, err
}
y, err := e.evaluate(yp)
if err != nil {
return nil, err
}
eq, err := rawEquals(e, x, y)
if err != nil {
return nil, err
}
return makeValueBoolean(!eq), nil
}

func builtinType(e *evaluator, xp potentialValue) (value, error) {
x, err := e.evaluate(xp)
if err != nil {
Expand Down Expand Up @@ -829,10 +956,8 @@ func (b *ternaryBuiltin) Name() ast.Identifier {
}

var desugaredBop = map[ast.BinaryOp]ast.Identifier{
ast.BopPercent: "mod",
ast.BopManifestEqual: "equals",
ast.BopManifestUnequal: "notEquals", // Special case
ast.BopIn: "objectHasAll",
ast.BopPercent: "mod",
ast.BopIn: "objectHasAll",
}

var bopBuiltins = []*binaryBuiltin{
Expand All @@ -851,8 +976,8 @@ var bopBuiltins = []*binaryBuiltin{
ast.BopLess: &binaryBuiltin{name: "operator<,", function: builtinLess, parameters: ast.Identifiers{"x", "y"}},
ast.BopLessEq: &binaryBuiltin{name: "operator<=", function: builtinLessEq, parameters: ast.Identifiers{"x", "y"}},

// bopManifestEqual: <desugared>,
// bopManifestUnequal: <desugared>,
ast.BopManifestEqual: &binaryBuiltin{name: "operator==", function: builtinEquals, parameters: ast.Identifiers{"x", "y"}},
ast.BopManifestUnequal: &binaryBuiltin{name: "operator!=", function: builtinNotEquals, parameters: ast.Identifiers{"x", "y"}}, // Special case

ast.BopBitwiseAnd: &binaryBuiltin{name: "operator&", function: builtinBitwiseAnd, parameters: ast.Identifiers{"x", "y"}},
ast.BopBitwiseXor: &binaryBuiltin{name: "operator^", function: builtinBitwiseXor, parameters: ast.Identifiers{"x", "y"}},
Expand Down Expand Up @@ -892,7 +1017,8 @@ var funcBuiltins = buildBuiltinMap([]builtin{
&binaryBuiltin{name: "join", function: builtinJoin, parameters: ast.Identifiers{"sep", "arr"}},
&binaryBuiltin{name: "filter", function: builtinFilter, parameters: ast.Identifiers{"func", "arr"}},
&binaryBuiltin{name: "range", function: builtinRange, parameters: ast.Identifiers{"from", "to"}},
&binaryBuiltin{name: "primitiveEquals", function: primitiveEquals, parameters: ast.Identifiers{"sz", "func"}},
&binaryBuiltin{name: "primitiveEquals", function: primitiveEquals, parameters: ast.Identifiers{"x", "y"}},
&binaryBuiltin{name: "equals", function: builtinEquals, parameters: ast.Identifiers{"x", "y"}},
&binaryBuiltin{name: "objectFieldsEx", function: builtinObjectFieldsEx, parameters: ast.Identifiers{"obj", "hidden"}},
&ternaryBuiltin{name: "objectHasEx", function: builtinObjectHasEx, parameters: ast.Identifiers{"obj", "fname", "hidden"}},
&unaryBuiltin{name: "type", function: builtinType, parameters: ast.Identifiers{"x"}},
Expand Down
9 changes: 1 addition & 8 deletions desugarer.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,14 +378,7 @@ func desugar(astPtr *ast.Node, objLevel int) (err error) {
case *ast.Binary:
// some operators get replaced by stdlib functions
if funcname, replaced := desugaredBop[node.Op]; replaced {
if funcname == "notEquals" {
// TODO(sbarzowski) maybe we can handle it in more regular way
// but let's be consistent with the spec
*astPtr = &ast.Unary{
Op: ast.UopNot,
Expr: buildStdCall(desugaredBop[ast.BopManifestEqual], node.Left, node.Right),
}
} else if node.Op == ast.BopIn {
if node.Op == ast.BopIn {
// reversed order of arguments
*astPtr = buildStdCall(funcname, node.Right, node.Left)
} else {
Expand Down
1 change: 1 addition & 0 deletions testdata/equals2.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
false
1 change: 1 addition & 0 deletions testdata/equals2.jsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{x: 1, y: 1} == {x: 2, y: 2}
1 change: 1 addition & 0 deletions testdata/equals3.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
true
1 change: 1 addition & 0 deletions testdata/equals3.jsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{x: 1} == {x: 1}
1 change: 1 addition & 0 deletions testdata/equals4.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
false
1 change: 1 addition & 0 deletions testdata/equals4.jsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{x: 1, y: 1} == {x: 1, y: 1, z: 2}
1 change: 1 addition & 0 deletions testdata/equals5.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
true
1 change: 1 addition & 0 deletions testdata/equals5.jsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{x: 1, y: 1} == {x: 1, y: 1, z:: 2}
1 change: 1 addition & 0 deletions testdata/equals6.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
true
2 changes: 2 additions & 0 deletions testdata/equals6.jsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
local data = {[std.toString(x)]: 1 for x in std.range(1, 100)};
data == data
27 changes: 2 additions & 25 deletions testdata/object_invariant7.golden
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,9 @@ RUNTIME ERROR: Attempt to use super when there is no super class.
{ x: 5, assert super.x == 5 }

-------------------------------------------------
<std>:1188:25-26 thunk from <thunk <ta> from <function <anonymous>>>
testdata/object_invariant7:1:16-28 object <anonymous>

local ta = std.type(a);

-------------------------------------------------
<std>:1188:16-27 builtin function <type>

local ta = std.type(a);

-------------------------------------------------
<std>:1190:29-31 thunk from <function <anonymous>>

if !std.primitiveEquals(ta, tb) then

-------------------------------------------------
<std>:1190:9-36 builtin function <primitiveEquals>

if !std.primitiveEquals(ta, tb) then

-------------------------------------------------
<std>:1190:8-36 function <anonymous>

if !std.primitiveEquals(ta, tb) then

-------------------------------------------------

{ x: 5, assert super.x == 5 }

-------------------------------------------------
During manifestation
Expand Down
1 change: 1 addition & 0 deletions value.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,7 @@ func objectFieldsVisibility(obj valueObject) fieldHideMap {
return r
}

// Returns field names of an object. Gotcha: the order of fields is unpredictable.
func objectFields(obj valueObject, h Hidden) []string {
var r []string
for fieldName, hide := range objectFieldsVisibility(obj) {
Expand Down

0 comments on commit 530cf7b

Please sign in to comment.