From 89db19a546238d087cb09160417e9cff36097667 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 22998aa5..c72a2baf 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 2d47018c..209072fd 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 @@ -187,6 +243,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 @@ -244,6 +314,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 b45ef0810c7373caf177b736bcde86bc59990cbc 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 | 163 +++++++++++++++++++++++++++------------------ 2 files changed, 113 insertions(+), 63 deletions(-) diff --git a/value/less_test.go b/value/less_test.go index c72a2baf..c44ff4f1 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.a) { 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 209072fd..942d7972 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 @@ -207,31 +244,31 @@ func (f FieldList) Sort() { // Less compares two lists lexically. func (f FieldList) Less(rhs FieldList) bool { + return f.Compare(rhs) == -1 +} + +// Less compares two lists lexically. The result will be 0 if f==rhs, -1 +// if f < rhs, and +1 if f > rhs. +func (f FieldList) Compare(rhs FieldList) int { i := 0 for { if i >= len(f) && i >= len(rhs) { // Maps are the same length and all items are equal. - return false + return 0 } if i >= len(f) { // F is shorter. - return true + return -1 } if i >= len(rhs) { // RHS is shorter. - return false + return 1 } - if f[i].Name != rhs[i].Name { - // the map having the field name that sorts lexically less is "less" - return f[i].Name < rhs[i].Name + if c := strings.Compare(f[i].Name, rhs[i].Name); c != 0 { + return c } - if f[i].Value.Less(rhs[i].Value) { - // F is less; return - return true - } - if rhs[i].Value.Less(f[i].Value) { - // RHS is less; return - return false + if c := f[i].Value.Compare(rhs[i].Value); c != 0 { + return c } // The items are equal; continue. i++ @@ -259,27 +296,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++ @@ -333,6 +371,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 @@ -378,28 +421,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++