Skip to content

Commit 602f4c3

Browse files
jiahuifjiahuif
committed
use a "seen" set instead of index.
1 parent 249e316 commit 602f4c3

File tree

3 files changed

+31
-36
lines changed

3 files changed

+31
-36
lines changed

‎fieldpath/pathelementmap.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ func MakePathElementValueMap(size int) PathElementValueMap {
4040
type pathElementValue struct {
4141
PathElement PathElement
4242
Value value.Value
43-
Index int
4443
}
4544

4645
type sortedPathElementValues []pathElementValue
@@ -49,42 +48,38 @@ type sortedPathElementValues []pathElementValue
4948
// be faster than doing it one at a time via Insert.
5049
func (spev sortedPathElementValues) Len() int { return len(spev) }
5150
func (spev sortedPathElementValues) Less(i, j int) bool {
52-
cmp := spev[i].PathElement.Compare(spev[j].PathElement)
53-
if cmp != 0 {
54-
return cmp < 0
55-
}
56-
return spev[i].Index < spev[j].Index
51+
return spev[i].PathElement.Less(spev[j].PathElement)
5752
}
5853
func (spev sortedPathElementValues) Swap(i, j int) { spev[i], spev[j] = spev[j], spev[i] }
5954

6055
// Insert adds the pathelement and associated value in the map.
61-
func (s *PathElementValueMap) Insert(pe PathElement, v value.Value, index int) {
56+
func (s *PathElementValueMap) Insert(pe PathElement, v value.Value) {
6257
loc := sort.Search(len(s.members), func(i int) bool {
6358
return !s.members[i].PathElement.Less(pe)
6459
})
6560
if loc == len(s.members) {
66-
s.members = append(s.members, pathElementValue{pe, v, index})
61+
s.members = append(s.members, pathElementValue{pe, v})
6762
return
6863
}
6964
if s.members[loc].PathElement.Equals(pe) {
7065
return
7166
}
7267
s.members = append(s.members, pathElementValue{})
7368
copy(s.members[loc+1:], s.members[loc:])
74-
s.members[loc] = pathElementValue{pe, v, index}
69+
s.members[loc] = pathElementValue{pe, v}
7570
}
7671

7772
// Get retrieves the value associated with the given PathElement from the map.
7873
// (nil, false) is returned if there is no such PathElement.
79-
func (s *PathElementValueMap) Get(pe PathElement) (value value.Value, index int, found bool) {
74+
func (s *PathElementValueMap) Get(pe PathElement) (value.Value, bool) {
8075
loc := sort.Search(len(s.members), func(i int) bool {
8176
return !s.members[i].PathElement.Less(pe)
8277
})
8378
if loc == len(s.members) {
84-
return nil, 0, false
79+
return nil, false
8580
}
8681
if s.members[loc].PathElement.Equals(pe) {
87-
return s.members[loc].Value, s.members[loc].Index, true
82+
return s.members[loc].Value, true
8883
}
89-
return nil, 0, false
84+
return nil, false
9085
}

‎fieldpath/pathelementmap_test.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,30 +25,26 @@ import (
2525
func TestPathElementValueMap(t *testing.T) {
2626
m := PathElementValueMap{}
2727

28-
if _, _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
28+
if _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
2929
t.Fatal("Unexpected path-element found in empty map")
3030
}
3131

32-
m.Insert(PathElement{FieldName: strptr("carrot")}, value.NewValueInterface("knife"), 1)
33-
m.Insert(PathElement{FieldName: strptr("chive")}, value.NewValueInterface(2), 2)
32+
m.Insert(PathElement{FieldName: strptr("carrot")}, value.NewValueInterface("knife"))
33+
m.Insert(PathElement{FieldName: strptr("chive")}, value.NewValueInterface(2))
3434

35-
if _, _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
35+
if _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
3636
t.Fatal("Unexpected path-element in map")
3737
}
3838

39-
if val, index, ok := m.Get(PathElement{FieldName: strptr("carrot")}); !ok {
39+
if val, ok := m.Get(PathElement{FieldName: strptr("carrot")}); !ok {
4040
t.Fatal("Missing path-element in map")
4141
} else if !value.Equals(val, value.NewValueInterface("knife")) {
4242
t.Fatalf("Unexpected value found: %#v", val)
43-
} else if index != 1 {
44-
t.Fatalf("Unexpected index found: %#v", index)
4543
}
4644

47-
if val, index, ok := m.Get(PathElement{FieldName: strptr("chive")}); !ok {
45+
if val, ok := m.Get(PathElement{FieldName: strptr("chive")}); !ok {
4846
t.Fatal("Missing path-element in map")
4947
} else if !value.Equals(val, value.NewValueInterface(2)) {
5048
t.Fatalf("Unexpected value found: %#v", val)
51-
} else if index != 2 {
52-
t.Fatalf("Unexpected index found: %#v", index)
5349
}
5450
}

‎typed/merge.go

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ limitations under the License.
1717
package typed
1818

1919
import (
20-
"math"
21-
2220
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
2321
"sigs.k8s.io/structured-merge-diff/v4/schema"
2422
"sigs.k8s.io/structured-merge-diff/v4/value"
@@ -170,7 +168,11 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
170168
if lhs != nil {
171169
lLen = lhs.Length()
172170
}
173-
out := make([]interface{}, 0, int(math.Max(float64(rLen), float64(lLen))))
171+
outLen := lLen
172+
if outLen < rLen {
173+
outLen = rLen
174+
}
175+
out := make([]interface{}, 0, outLen)
174176

175177
createPathElementsValues := func(name string, list value.List) ([]fieldpath.PathElement, fieldpath.PathElementValueMap, ValidationErrors) {
176178
var errs ValidationErrors
@@ -190,11 +192,11 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
190192
// this element.
191193
continue
192194
}
193-
if _, _, found := observed.Get(pe); found {
195+
if _, found := observed.Get(pe); found {
194196
errs = append(errs, errorf("%s: duplicate entries for key %v", name, pe.String())...)
195197
continue
196198
}
197-
observed.Insert(pe, child, i)
199+
observed.Insert(pe, child)
198200
pes = append(pes, pe)
199201
}
200202
return pes, observed, errs
@@ -204,6 +206,7 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
204206
errs = append(errs, lhsErrs...)
205207
rhsOrder, observedRHS, rhsErrs := createPathElementsValues("rhs", rhs)
206208
errs = append(errs, rhsErrs...)
209+
seen := fieldpath.MakePathElementSet(outLen)
207210

208211
lLen, rLen = len(lhsOrder), len(rhsOrder)
209212
for lI, rI := 0, 0; lI < lLen || rI < rLen; {
@@ -216,26 +219,27 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
216219
out = append(out, *w2.out)
217220
}
218221
w.finishDescent(w2)
222+
seen.Insert(pe)
219223
}
220224
if lI < lLen && rI < rLen && lhsOrder[lI].Equals(rhsOrder[rI]) {
221225
// merge LHS & RHS items
222226
pe := lhsOrder[lI]
223-
lChild, _, _ := observedLHS.Get(pe)
224-
rChild, _, _ := observedRHS.Get(pe)
227+
lChild, _ := observedLHS.Get(pe)
228+
rChild, _ := observedRHS.Get(pe)
225229
merge(pe, lChild, rChild)
226230
lI++
227231
rI++
228232
continue
229233
}
230234
if lI < lLen {
231-
if _, index, ok := observedRHS.Get(lhsOrder[lI]); ok && index < rI {
235+
pe := lhsOrder[lI]
236+
if ok := seen.Has(pe); ok {
232237
// Skip the LHS item because it has already appeared
233238
lI++
234239
continue
235-
} else if !ok {
240+
} else if _, ok := observedRHS.Get(pe); !ok {
236241
// Take the LHS item, without a matching RHS item to merge with
237-
pe := lhsOrder[lI]
238-
lChild, _, _ := observedLHS.Get(pe)
242+
lChild, _ := observedLHS.Get(pe)
239243
merge(pe, lChild, nil)
240244
lI++
241245
continue
@@ -244,8 +248,8 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
244248
if rI < rLen {
245249
// Take the RHS item, merge with matching LHS item if possible
246250
pe := rhsOrder[rI]
247-
rChild, _, _ := observedRHS.Get(pe)
248-
lChild, _, _ := observedLHS.Get(pe) // may be nil
251+
rChild, _ := observedRHS.Get(pe)
252+
lChild, _ := observedLHS.Get(pe) // may be nil
249253
merge(pe, lChild, rChild)
250254
rI++
251255
}

0 commit comments

Comments
 (0)