From fa5e197e5401262a78fa515069807d71c5da3d4c Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Tue, 5 Nov 2019 16:41:21 -0800 Subject: [PATCH 1/2] Implement equality for Values Equality doesn't require the same amount of sophistication than ordering comparison. Implement an Equal function that does just that but does it much faster. Benchmark results show up to 96% improvement: ``` benchmark old ns/op new ns/op delta BenchmarkDeducedSimple-12 110083 103256 -6.20% BenchmarkDeducedNested-12 374366 374160 -0.06% BenchmarkDeducedNestedAcrossVersion-12 392864 405525 +3.22% BenchmarkLeafConflictAcrossVersion-12 89112 89070 -0.05% BenchmarkMultipleApplierRecursiveRealConversion-12 1564330 1574620 +0.66% BenchmarkOperations/Pod/Create-12 103693 103970 +0.27% BenchmarkOperations/Pod/Apply-12 291760 291317 -0.15% BenchmarkOperations/Pod/Update-12 193419 190470 -1.52% BenchmarkOperations/Pod/UpdateVersion-12 261692 251966 -3.72% BenchmarkOperations/Node/Create-12 152047 155710 +2.41% BenchmarkOperations/Node/Apply-12 499187 473901 -5.07% BenchmarkOperations/Node/Update-12 299271 279142 -6.73% BenchmarkOperations/Node/UpdateVersion-12 438723 403125 -8.11% BenchmarkOperations/Endpoints/Create-12 12246 11940 -2.50% BenchmarkOperations/Endpoints/Apply-12 915806 924080 +0.90% BenchmarkOperations/Endpoints/Update-12 7155675 285092 -96.02% BenchmarkOperations/Endpoints/UpdateVersion-12 14278150 544040 -96.19% BenchmarkOperations/CustomResourceDefinition/Create-12 1312734 1288472 -1.85% BenchmarkOperations/CustomResourceDefinition/Apply-12 3346591 3376864 +0.90% BenchmarkOperations/CustomResourceDefinition/Update-12 10681243 1758764 -83.53% BenchmarkOperations/CustomResourceDefinition/UpdateVersion-12 19069925 2202330 -88.45% ``` --- value/less_test.go | 3 ++ value/value.go | 89 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/value/less_test.go b/value/less_test.go index b0b15f37..3c667b07 100644 --- a/value/less_test.go +++ b/value/less_test.go @@ -296,6 +296,9 @@ func TestValueLess(t *testing.T) { t.Run(table[i].name, func(t *testing.T) { tt := table[i] if tt.eq { + if !tt.a.Equals(tt.b) { + t.Errorf("oops, a != b: %#v, %#v", tt.a, tt.b) + } if tt.a.Less(tt.b) { t.Errorf("oops, a < b: %#v, %#v", tt.a, tt.b) } diff --git a/value/value.go b/value/value.go index 1ce63e1c..8c418419 100644 --- a/value/value.go +++ b/value/value.go @@ -36,7 +36,63 @@ type Value struct { // Equals returns true iff the two values are equal. func (v Value) Equals(rhs Value) bool { - return !v.Less(rhs) && !rhs.Less(v) + if v.FloatValue != nil || rhs.FloatValue != nil { + var lf float64 + if v.FloatValue != nil { + lf = float64(*v.FloatValue) + } else if v.IntValue != nil { + lf = float64(*v.IntValue) + } else { + return false + } + var rf float64 + if rhs.FloatValue != nil { + rf = float64(*rhs.FloatValue) + } else if rhs.IntValue != nil { + rf = float64(*rhs.IntValue) + } else { + return false + } + return lf == rf + } + if v.IntValue != nil { + if rhs.IntValue != nil { + return *v.IntValue == *rhs.IntValue + } + return false + } + if v.StringValue != nil { + if rhs.StringValue != nil { + return *v.StringValue == *rhs.StringValue + } + return false + } + if v.BooleanValue != nil { + if rhs.BooleanValue != nil { + return *v.BooleanValue == *rhs.BooleanValue + } + return false + } + if v.ListValue != nil { + if rhs.ListValue != nil { + return v.ListValue.Equals(rhs.ListValue) + } + return false + } + if v.MapValue != nil { + if rhs.MapValue != nil { + return v.MapValue.Equals(rhs.MapValue) + } + return false + } + if v.Null { + if rhs.Null { + return true + } + return false + } + // No field is set, on either objects. + return true } // Less provides a total ordering for Value (so that they can be sorted, even @@ -134,6 +190,20 @@ type List struct { Items []Value } +// Equals compares two lists lexically. +func (l *List) Equals(rhs *List) bool { + if len(l.Items) != len(rhs.Items) { + return false + } + + for i, lv := range l.Items { + if !lv.Equals(rhs.Items[i]) { + return false + } + } + return true +} + // Less compares two lists lexically. func (l *List) Less(rhs *List) bool { i := 0 @@ -191,6 +261,23 @@ func (m *Map) computeOrder() []int { return m.order } +// Equals compares two maps lexically. +func (m *Map) Equals(rhs *Map) bool { + if len(m.Items) != len(rhs.Items) { + return false + } + for _, lfield := range m.Items { + rfield, ok := rhs.Get(lfield.Name) + if !ok { + return false + } + if !lfield.Value.Equals(rfield.Value) { + return false + } + } + return true +} + // Less compares two maps lexically. func (m *Map) Less(rhs *Map) bool { var noAllocL, noAllocR [2]int From 08cde4d5c832b74b2694f1ae609f67418255467e Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Tue, 5 Nov 2019 21:35:37 -0800 Subject: [PATCH 2/2] Implement Compare methods The current Less implementation has an exponential complexity. Replacing Less with a Compare method avoids that problem. --- value/less_test.go | 13 +++++ value/value.go | 137 ++++++++++++++++++++++++++++----------------- 2 files changed, 100 insertions(+), 50 deletions(-) diff --git a/value/less_test.go b/value/less_test.go index 3c667b07..7f568e7f 100644 --- a/value/less_test.go +++ b/value/less_test.go @@ -310,6 +310,19 @@ func TestValueLess(t *testing.T) { if tt.b.Less(tt.b) { t.Errorf("oops, b < a: %#v, %#v", tt.b, tt.a) } + + if tt.eq { + if tt.a.Compare(tt.b) != 0 || tt.b.Compare(tt.b) != 0 { + t.Errorf("oops, a != b: %#v, %#v", tt.a, tt.b) + } + } else { + if !(tt.a.Compare(tt.b) < 0) { + t.Errorf("oops, a is not less than b: %#v, %#v", tt.a, tt.b) + } + if !(tt.b.Compare(tt.a) > 0) { + t.Errorf("oops, b is not more than a: %#v, %#v", tt.a, tt.b) + } + } }) } diff --git a/value/value.go b/value/value.go index 8c418419..91d73d0f 100644 --- a/value/value.go +++ b/value/value.go @@ -98,80 +98,84 @@ func (v Value) Equals(rhs Value) bool { // Less provides a total ordering for Value (so that they can be sorted, even // if they are of different types). func (v Value) Less(rhs Value) bool { + return v.Compare(rhs) == -1 +} + +// Compare provides a total ordering for Value (so that they can be +// sorted, even if they are of different types). The result will be 0 if +// v==rhs, -1 if v < rhs, and +1 if v > rhs. +func (v Value) Compare(rhs Value) int { if v.FloatValue != nil { if rhs.FloatValue == nil { // Extra: compare floats and ints numerically. if rhs.IntValue != nil { - return float64(*v.FloatValue) < float64(*rhs.IntValue) + return v.FloatValue.Compare(Float(*rhs.IntValue)) } - return true + return -1 } - return *v.FloatValue < *rhs.FloatValue + return v.FloatValue.Compare(*rhs.FloatValue) } else if rhs.FloatValue != nil { // Extra: compare floats and ints numerically. if v.IntValue != nil { - return float64(*v.IntValue) < float64(*rhs.FloatValue) + return Float(*v.IntValue).Compare(*rhs.FloatValue) } - return false + return 1 } if v.IntValue != nil { if rhs.IntValue == nil { - return true + return -1 } - return *v.IntValue < *rhs.IntValue + return v.IntValue.Compare(*rhs.IntValue) } else if rhs.IntValue != nil { - return false + return 1 } if v.StringValue != nil { if rhs.StringValue == nil { - return true + return -1 } - return *v.StringValue < *rhs.StringValue + return strings.Compare(string(*v.StringValue), string(*rhs.StringValue)) } else if rhs.StringValue != nil { - return false + return 1 } if v.BooleanValue != nil { if rhs.BooleanValue == nil { - return true - } - if *v.BooleanValue == *rhs.BooleanValue { - return false + return -1 } - return *v.BooleanValue == false + return v.BooleanValue.Compare(*rhs.BooleanValue) } else if rhs.BooleanValue != nil { - return false + return 1 } if v.ListValue != nil { if rhs.ListValue == nil { - return true + return -1 } - return v.ListValue.Less(rhs.ListValue) + return v.ListValue.Compare(rhs.ListValue) } else if rhs.ListValue != nil { - return false + return 1 } if v.MapValue != nil { if rhs.MapValue == nil { - return true + return -1 } - return v.MapValue.Less(rhs.MapValue) + return v.MapValue.Compare(rhs.MapValue) } else if rhs.MapValue != nil { - return false + return 1 } if v.Null { if !rhs.Null { - return true + return -1 } - return false + return 0 } else if rhs.Null { - return false + return 1 } // Invalid Value-- nothing is set. - return false + return 0 } type Int int64 @@ -179,6 +183,39 @@ type Float float64 type String string type Boolean bool +// Compare compares integers. The result will be 0 if i==rhs, -1 if i < +// rhs, and +1 if i > rhs. +func (i Int) Compare(rhs Int) int { + if i > rhs { + return 1 + } else if i < rhs { + return -1 + } + return 0 +} + +// Compare compares floats. The result will be 0 if f==rhs, -1 if f < +// rhs, and +1 if f > rhs. +func (f Float) Compare(rhs Float) int { + if f > rhs { + return 1 + } else if f < rhs { + return -1 + } + return 0 +} + +// Compare compares booleans. The result will be 0 if b==rhs, -1 if b < +// rhs, and +1 if b > rhs. +func (b Boolean) Compare(rhs Boolean) int { + if b == rhs { + return 0 + } else if b == false { + return -1 + } + return 1 +} + // Field is an individual key-value pair. type Field struct { Name string @@ -206,27 +243,28 @@ func (l *List) Equals(rhs *List) bool { // Less compares two lists lexically. func (l *List) Less(rhs *List) bool { + return l.Compare(rhs) == -1 +} + +// Compare compares two lists lexically. The result will be 0 if l==rhs, -1 +// if l < rhs, and +1 if l > rhs. +func (l *List) Compare(rhs *List) int { i := 0 for { if i >= len(l.Items) && i >= len(rhs.Items) { // Lists are the same length and all items are equal. - return false + return 0 } if i >= len(l.Items) { // LHS is shorter. - return true + return -1 } if i >= len(rhs.Items) { // RHS is shorter. - return false + return 1 } - if l.Items[i].Less(rhs.Items[i]) { - // LHS is less; return - return true - } - if rhs.Items[i].Less(l.Items[i]) { - // RHS is less; return - return false + if c := l.Items[i].Compare(rhs.Items[i]); c != 0 { + return c } // The items are equal; continue. i++ @@ -280,6 +318,11 @@ func (m *Map) Equals(rhs *Map) bool { // Less compares two maps lexically. func (m *Map) Less(rhs *Map) bool { + return m.Compare(rhs) == -1 +} + +// Compare compares two maps lexically. +func (m *Map) Compare(rhs *Map) int { var noAllocL, noAllocR [2]int var morder, rorder []int @@ -325,28 +368,22 @@ func (m *Map) Less(rhs *Map) bool { for { if i >= len(morder) && i >= len(rorder) { // Maps are the same length and all items are equal. - return false + return 0 } if i >= len(morder) { // LHS is shorter. - return true + return -1 } if i >= len(rorder) { // RHS is shorter. - return false + return 1 } fa, fb := &m.Items[morder[i]], &rhs.Items[rorder[i]] - if fa.Name != fb.Name { - // the map having the field name that sorts lexically less is "less" - return fa.Name < fb.Name + if c := strings.Compare(fa.Name, fb.Name); c != 0 { + return c } - if fa.Value.Less(fb.Value) { - // LHS is less; return - return true - } - if fb.Value.Less(fa.Value) { - // RHS is less; return - return false + if c := fa.Value.Compare(fb.Value); c != 0 { + return c } // The items are equal; continue. i++