Skip to content

Commit f7547e8

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#88 from lavalamp/optimize
Optimize benchmarks; permit serialization
2 parents eaa53bf + 03f010c commit f7547e8

File tree

15 files changed

+1049
-98
lines changed

15 files changed

+1049
-98
lines changed

‎fieldpath/element.go

Lines changed: 132 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type PathElement struct {
3535
// Key selects the list element which has fields matching those given.
3636
// The containing object must be an associative list with map typed
3737
// elements.
38-
Key []value.Field
38+
Key *value.Map
3939

4040
// Value selects the list element with the given value. The containing
4141
// object must be an associative list with a primitive typed element
@@ -47,14 +47,62 @@ type PathElement struct {
4747
Index *int
4848
}
4949

50+
// Less provides an order for path elements.
51+
func (e PathElement) Less(rhs PathElement) bool {
52+
if e.FieldName != nil {
53+
if rhs.FieldName == nil {
54+
return true
55+
}
56+
return *e.FieldName < *rhs.FieldName
57+
} else if rhs.FieldName != nil {
58+
return false
59+
}
60+
61+
if e.Key != nil {
62+
if rhs.Key == nil {
63+
return true
64+
}
65+
return e.Key.Less(rhs.Key)
66+
} else if rhs.Key != nil {
67+
return false
68+
}
69+
70+
if e.Value != nil {
71+
if rhs.Value == nil {
72+
return true
73+
}
74+
return e.Value.Less(*rhs.Value)
75+
} else if rhs.Value != nil {
76+
return false
77+
}
78+
79+
if e.Index != nil {
80+
if rhs.Index == nil {
81+
return true
82+
}
83+
return *e.Index < *rhs.Index
84+
} else if rhs.Index != nil {
85+
// Yes, I know the next statement is the same. But this way
86+
// the obvious way of extending the function wil be bug-free.
87+
return false
88+
}
89+
90+
return false
91+
}
92+
93+
// Equals returns true if both path elements are equal.
94+
func (e PathElement) Equals(rhs PathElement) bool {
95+
return !e.Less(rhs) && !rhs.Less(e)
96+
}
97+
5098
// String presents the path element as a human-readable string.
5199
func (e PathElement) String() string {
52100
switch {
53101
case e.FieldName != nil:
54102
return "." + *e.FieldName
55-
case len(e.Key) > 0:
56-
strs := make([]string, len(e.Key))
57-
for i, k := range e.Key {
103+
case e.Key != nil:
104+
strs := make([]string, len(e.Key.Items))
105+
for i, k := range e.Key.Items {
58106
strs[i] = fmt.Sprintf("%v=%v", k.Name, k.Value)
59107
}
60108
// The order must be canonical, since we use the string value
@@ -92,62 +140,104 @@ func KeyByFields(nameValues ...interface{}) []value.Field {
92140
// PathElementSet is a set of path elements.
93141
// TODO: serialize as a list.
94142
type PathElementSet struct {
95-
// The strange construction is because there's no way to test
96-
// PathElements for equality (it can't be used as a key for a map).
97-
members map[string]PathElement
143+
members sortedPathElements
98144
}
99145

146+
type sortedPathElements []PathElement
147+
148+
// Implement the sort interface; this would permit bulk creation, which would
149+
// be faster than doing it one at a time via Insert.
150+
func (spe sortedPathElements) Len() int { return len(spe) }
151+
func (spe sortedPathElements) Less(i, j int) bool { return spe[i].Less(spe[j]) }
152+
func (spe sortedPathElements) Swap(i, j int) { spe[i], spe[j] = spe[j], spe[i] }
153+
100154
// Insert adds pe to the set.
101155
func (s *PathElementSet) Insert(pe PathElement) {
102-
serialized := pe.String()
103-
if s.members == nil {
104-
s.members = map[string]PathElement{
105-
serialized: pe,
106-
}
156+
loc := sort.Search(len(s.members), func(i int) bool {
157+
return !s.members[i].Less(pe)
158+
})
159+
if loc == len(s.members) {
160+
s.members = append(s.members, pe)
107161
return
108162
}
109-
if _, ok := s.members[serialized]; !ok {
110-
s.members[serialized] = pe
163+
if s.members[loc].Equals(pe) {
164+
return
165+
}
166+
n := len(s.members) - 1
167+
s.members = append(s.members, s.members[n])
168+
for n > loc {
169+
s.members[n] = s.members[n-1]
170+
n--
111171
}
172+
s.members[loc] = pe
112173
}
113174

114175
// Union returns a set containing elements that appear in either s or s2.
115176
func (s *PathElementSet) Union(s2 *PathElementSet) *PathElementSet {
116-
out := &PathElementSet{
117-
members: map[string]PathElement{},
177+
out := &PathElementSet{}
178+
179+
i, j := 0, 0
180+
for i < len(s.members) && j < len(s2.members) {
181+
if s.members[i].Less(s2.members[j]) {
182+
out.members = append(out.members, s.members[i])
183+
i++
184+
} else {
185+
out.members = append(out.members, s2.members[j])
186+
if !s2.members[j].Less(s.members[i]) {
187+
i++
188+
}
189+
j++
190+
}
118191
}
119-
for k, v := range s.members {
120-
out.members[k] = v
192+
193+
if i < len(s.members) {
194+
out.members = append(out.members, s.members[i:]...)
121195
}
122-
for k, v := range s2.members {
123-
out.members[k] = v
196+
if j < len(s2.members) {
197+
out.members = append(out.members, s2.members[j:]...)
124198
}
125199
return out
126200
}
127201

128202
// Intersection returns a set containing elements which appear in both s and s2.
129203
func (s *PathElementSet) Intersection(s2 *PathElementSet) *PathElementSet {
130-
out := &PathElementSet{
131-
members: map[string]PathElement{},
132-
}
133-
for k, v := range s.members {
134-
if _, ok := s2.members[k]; ok {
135-
out.members[k] = v
204+
out := &PathElementSet{}
205+
206+
i, j := 0, 0
207+
for i < len(s.members) && j < len(s2.members) {
208+
if s.members[i].Less(s2.members[j]) {
209+
i++
210+
} else {
211+
if !s2.members[j].Less(s.members[i]) {
212+
out.members = append(out.members, s.members[i])
213+
i++
214+
}
215+
j++
136216
}
137217
}
218+
138219
return out
139220
}
140221

141222
// Difference returns a set containing elements which appear in s but not in s2.
142223
func (s *PathElementSet) Difference(s2 *PathElementSet) *PathElementSet {
143-
out := &PathElementSet{
144-
members: map[string]PathElement{},
145-
}
146-
for k, v := range s.members {
147-
if _, ok := s2.members[k]; !ok {
148-
out.members[k] = v
224+
out := &PathElementSet{}
225+
226+
i, j := 0, 0
227+
for i < len(s.members) && j < len(s2.members) {
228+
if s.members[i].Less(s2.members[j]) {
229+
out.members = append(out.members, s.members[i])
230+
i++
231+
} else {
232+
if !s2.members[j].Less(s.members[i]) {
233+
i++
234+
}
235+
j++
149236
}
150237
}
238+
if i < len(s.members) {
239+
out.members = append(out.members, s.members[i:]...)
240+
}
151241
return out
152242
}
153243

@@ -156,11 +246,16 @@ func (s *PathElementSet) Size() int { return len(s.members) }
156246

157247
// Has returns true if pe is a member of the set.
158248
func (s *PathElementSet) Has(pe PathElement) bool {
159-
if s.members == nil {
249+
loc := sort.Search(len(s.members), func(i int) bool {
250+
return !s.members[i].Less(pe)
251+
})
252+
if loc == len(s.members) {
160253
return false
161254
}
162-
_, ok := s.members[pe.String()]
163-
return ok
255+
if s.members[loc].Equals(pe) {
256+
return true
257+
}
258+
return false
164259
}
165260

166261
// Equals returns true if s and s2 have exactly the same members.
@@ -169,14 +264,14 @@ func (s *PathElementSet) Equals(s2 *PathElementSet) bool {
169264
return false
170265
}
171266
for k := range s.members {
172-
if _, ok := s2.members[k]; !ok {
267+
if !s.members[k].Equals(s2.members[k]) {
173268
return false
174269
}
175270
}
176271
return true
177272
}
178273

179-
// Iterate calls f for each PathElement in the set.
274+
// Iterate calls f for each PathElement in the set. The order is deterministic.
180275
func (s *PathElementSet) Iterate(f func(PathElement)) {
181276
for _, pe := range s.members {
182277
f(pe)

‎fieldpath/element_test.go

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ package fieldpath
1818

1919
import (
2020
"testing"
21+
22+
"sigs.k8s.io/structured-merge-diff/value"
2123
)
2224

2325
func TestPathElementSet(t *testing.T) {
@@ -28,4 +30,142 @@ func TestPathElementSet(t *testing.T) {
2830
if s2.Equals(s) {
2931
t.Errorf("unequal sets should not equal")
3032
}
33+
if !s2.Has(PathElement{}) {
34+
t.Errorf("expected to have something: %#v", s2)
35+
}
36+
37+
n1 := "aoeu"
38+
n2 := "asdf"
39+
s2.Insert(PathElement{FieldName: &n1})
40+
if !s2.Has(PathElement{FieldName: &n1}) {
41+
t.Errorf("expected to have something: %#v", s2)
42+
}
43+
if s2.Has(PathElement{FieldName: &n2}) {
44+
t.Errorf("expected to not have something: %#v", s2)
45+
}
46+
47+
s2.Insert(PathElement{FieldName: &n2})
48+
expected := []*string{&n1, &n2, nil}
49+
i := 0
50+
s2.Iterate(func(pe PathElement) {
51+
e, a := expected[i], pe.FieldName
52+
if e == nil || a == nil {
53+
if e != a {
54+
t.Errorf("index %v wanted %#v, got %#v", i, e, a)
55+
}
56+
} else {
57+
if *e != *a {
58+
t.Errorf("index %v wanted %#v, got %#v", i, *e, *a)
59+
}
60+
}
61+
i++
62+
})
63+
}
64+
65+
func strptr(s string) *string { return &s }
66+
func intptr(i int) *int { return &i }
67+
func valptr(i int) *value.Value { v := value.IntValue(i); return &v }
68+
69+
func TestPathElementLess(t *testing.T) {
70+
table := []struct {
71+
name string
72+
// we expect a < b and !(b < a) unless eq is true, in which
73+
// case we expect less to return false in both orders.
74+
a, b PathElement
75+
eq bool
76+
}{
77+
{
78+
name: "FieldName-0",
79+
a: PathElement{},
80+
b: PathElement{},
81+
eq: true,
82+
}, {
83+
name: "FieldName-1",
84+
a: PathElement{FieldName: strptr("anteater")},
85+
b: PathElement{FieldName: strptr("zebra")},
86+
}, {
87+
name: "FieldName-2",
88+
a: PathElement{FieldName: strptr("bee")},
89+
b: PathElement{FieldName: strptr("bee")},
90+
eq: true,
91+
}, {
92+
name: "FieldName-3",
93+
a: PathElement{FieldName: strptr("capybara")},
94+
b: PathElement{Key: &value.Map{Items: []value.Field{{Name: "dog", Value: value.IntValue(3)}}}},
95+
}, {
96+
name: "FieldName-4",
97+
a: PathElement{FieldName: strptr("elephant")},
98+
b: PathElement{Value: valptr(4)},
99+
}, {
100+
name: "FieldName-5",
101+
a: PathElement{FieldName: strptr("falcon")},
102+
b: PathElement{Index: intptr(5)},
103+
}, {
104+
name: "Key-1",
105+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "goat", Value: value.IntValue(1)}}}},
106+
b: PathElement{Key: &value.Map{Items: []value.Field{{Name: "goat", Value: value.IntValue(1)}}}},
107+
eq: true,
108+
}, {
109+
name: "Key-2",
110+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "horse", Value: value.IntValue(1)}}}},
111+
b: PathElement{Key: &value.Map{Items: []value.Field{{Name: "horse", Value: value.IntValue(2)}}}},
112+
}, {
113+
name: "Key-3",
114+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "ibex", Value: value.IntValue(1)}}}},
115+
b: PathElement{Key: &value.Map{Items: []value.Field{{Name: "jay", Value: value.IntValue(1)}}}},
116+
}, {
117+
name: "Key-4",
118+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "kite", Value: value.IntValue(1)}}}},
119+
b: PathElement{Key: &value.Map{Items: []value.Field{{Name: "kite", Value: value.IntValue(1)}, {Name: "kite-2", Value: value.IntValue(1)}}}},
120+
}, {
121+
name: "Key-5",
122+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "kite", Value: value.IntValue(1)}}}},
123+
b: PathElement{Value: valptr(1)},
124+
}, {
125+
name: "Key-6",
126+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "kite", Value: value.IntValue(1)}}}},
127+
b: PathElement{Index: intptr(5)},
128+
}, {
129+
name: "Value-1",
130+
a: PathElement{Value: valptr(1)},
131+
b: PathElement{Value: valptr(2)},
132+
}, {
133+
name: "Value-2",
134+
a: PathElement{Value: valptr(1)},
135+
b: PathElement{Value: valptr(1)},
136+
eq: true,
137+
}, {
138+
name: "Value-3",
139+
a: PathElement{Value: valptr(1)},
140+
b: PathElement{Index: intptr(1)},
141+
}, {
142+
name: "Index-1",
143+
a: PathElement{Index: intptr(1)},
144+
b: PathElement{Index: intptr(2)},
145+
}, {
146+
name: "Index-2",
147+
a: PathElement{Index: intptr(1)},
148+
b: PathElement{Index: intptr(1)},
149+
eq: true,
150+
},
151+
}
152+
153+
for i := range table {
154+
i := i
155+
t.Run(table[i].name, func(t *testing.T) {
156+
tt := table[i]
157+
if tt.eq {
158+
if tt.a.Less(tt.b) {
159+
t.Errorf("oops, a < b: %#v, %#v", tt.a, tt.b)
160+
}
161+
} else {
162+
if !tt.a.Less(tt.b) {
163+
t.Errorf("oops, a >= b: %#v, %#v", tt.a, tt.b)
164+
}
165+
}
166+
if tt.b.Less(tt.b) {
167+
t.Errorf("oops, b < a: %#v, %#v", tt.b, tt.a)
168+
}
169+
})
170+
}
31171
}

0 commit comments

Comments
 (0)