Skip to content

Commit 13ba6db

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #113 from apelisse/perf/cache-schema-lists
Cache type information in Schema
2 parents 5a3d7b8 + cbd0d5f commit 13ba6db

File tree

17 files changed

+1000
-53
lines changed

17 files changed

+1000
-53
lines changed

‎go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module sigs.k8s.io/structured-merge-diff
33
require gopkg.in/yaml.v2 v2.2.1
44

55
require (
6+
github.com/google/gofuzz v1.0.0
67
github.com/json-iterator/go v1.1.6
78
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
89
github.com/modern-go/reflect2 v1.0.1 // indirect

‎go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
22
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
3+
github.com/google/gofuzz v1.0.0 h1:A8PeW59pxE9IoFRqBp37U+mSNaQoZ46F1f0f863XSXw=
4+
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
35
github.com/json-iterator/go v1.1.6 h1:MrUvLMLTMxbqFJ9kzlvat/rYZqZnW3u4wkLzWTaFwKs=
46
github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU=
57
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg=

‎schema/elements.go

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,17 @@ limitations under the License.
1616

1717
package schema
1818

19+
import "sync"
20+
1921
// Schema is a list of named types.
22+
//
23+
// Schema types are indexed in a map before the first search so this type
24+
// should be considered immutable.
2025
type Schema struct {
2126
Types []TypeDef `yaml:"types,omitempty"`
27+
28+
once sync.Once
29+
m map[string]TypeDef
2230
}
2331

2432
// A TypeSpecifier references a particular type in a schema.
@@ -92,6 +100,9 @@ const (
92100
//
93101
// Maps may also represent a type which is composed of a number of different fields.
94102
// Each field has a name and a type.
103+
//
104+
// Fields are indexed in a map before the first search so this type
105+
// should be considered immutable.
95106
type Map struct {
96107
// Each struct field appears exactly once in this list. The order in
97108
// this list defines the canonical field ordering.
@@ -117,6 +128,22 @@ type Map struct {
117128
// The default behavior for maps is `separable`; it's permitted to
118129
// leave this unset to get the default behavior.
119130
ElementRelationship ElementRelationship `yaml:"elementRelationship,omitempty"`
131+
132+
once sync.Once
133+
m map[string]StructField
134+
}
135+
136+
// FindField is a convenience function that returns the referenced StructField,
137+
// if it exists, or (nil, false) if it doesn't.
138+
func (m *Map) FindField(name string) (StructField, bool) {
139+
m.once.Do(func() {
140+
m.m = make(map[string]StructField, len(m.Fields))
141+
for _, field := range m.Fields {
142+
m.m[field.Name] = field
143+
}
144+
})
145+
sf, ok := m.m[name]
146+
return sf, ok
120147
}
121148

122149
// UnionFields are mapping between the fields that are part of the union and
@@ -204,13 +231,15 @@ type List struct {
204231

205232
// FindNamedType is a convenience function that returns the referenced TypeDef,
206233
// if it exists, or (nil, false) if it doesn't.
207-
func (s Schema) FindNamedType(name string) (TypeDef, bool) {
208-
for _, t := range s.Types {
209-
if t.Name == name {
210-
return t, true
234+
func (s *Schema) FindNamedType(name string) (TypeDef, bool) {
235+
s.once.Do(func() {
236+
s.m = make(map[string]TypeDef, len(s.Types))
237+
for _, t := range s.Types {
238+
s.m[t.Name] = t
211239
}
212-
}
213-
return TypeDef{}, false
240+
})
241+
t, ok := s.m[name]
242+
return t, ok
214243
}
215244

216245
// Resolve is a convenience function which returns the atom referenced, whether

‎schema/elements_test.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestFindNamedType(t *testing.T) {
4040
Types: tt.defs,
4141
}
4242
td, exist := s.FindNamedType(tt.namedType)
43-
if !reflect.DeepEqual(td, tt.expectTypeDef) {
43+
if !td.Equals(tt.expectTypeDef) {
4444
t.Errorf("expected TypeDef %v, got %v", tt.expectTypeDef, td)
4545
}
4646
if exist != tt.expectExist {
@@ -50,6 +50,43 @@ func TestFindNamedType(t *testing.T) {
5050
}
5151
}
5252

53+
func strptr(s string) *string { return &s }
54+
55+
func TestFindField(t *testing.T) {
56+
tests := []struct {
57+
testName string
58+
defs []StructField
59+
fieldName string
60+
expectStructField StructField
61+
expectExist bool
62+
}{
63+
{"existing", []StructField{
64+
{Name: "a", Type: TypeRef{NamedType: strptr("a")}},
65+
{Name: "b", Type: TypeRef{NamedType: strptr("b")}},
66+
}, "a", StructField{Name: "a", Type: TypeRef{NamedType: strptr("a")}}, true},
67+
{"notExisting", []StructField{
68+
{Name: "a", Type: TypeRef{NamedType: strptr("a")}},
69+
{Name: "b", Type: TypeRef{NamedType: strptr("b")}},
70+
}, "c", StructField{}, false},
71+
}
72+
for _, tt := range tests {
73+
tt := tt
74+
t.Run(tt.testName, func(t *testing.T) {
75+
t.Parallel()
76+
s := Map{
77+
Fields: tt.defs,
78+
}
79+
sf, exist := s.FindField(tt.fieldName)
80+
if !reflect.DeepEqual(sf, tt.expectStructField) {
81+
t.Errorf("expected StructField %v, got %v", tt.expectStructField, sf)
82+
}
83+
if exist != tt.expectExist {
84+
t.Errorf("expected existing %t, got %t", tt.expectExist, exist)
85+
}
86+
})
87+
}
88+
}
89+
5390
func TestResolve(t *testing.T) {
5491
existing := "existing"
5592
notExisting := "not-existing"

‎schema/equals_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,42 @@ limitations under the License.
1717
package schema
1818

1919
import (
20+
"math/rand"
2021
"reflect"
2122
"testing"
2223
"testing/quick"
24+
25+
fuzz "github.com/google/gofuzz"
2326
)
2427

28+
func (Schema) Generate(rand *rand.Rand, size int) reflect.Value {
29+
s := Schema{}
30+
f := fuzz.New().RandSource(rand).MaxDepth(4)
31+
f.Fuzz(&s)
32+
return reflect.ValueOf(s)
33+
}
34+
35+
func (Map) Generate(rand *rand.Rand, size int) reflect.Value {
36+
m := Map{}
37+
f := fuzz.New().RandSource(rand).MaxDepth(4)
38+
f.Fuzz(&m)
39+
return reflect.ValueOf(m)
40+
}
41+
42+
func (TypeDef) Generate(rand *rand.Rand, size int) reflect.Value {
43+
td := TypeDef{}
44+
f := fuzz.New().RandSource(rand).MaxDepth(4)
45+
f.Fuzz(&td)
46+
return reflect.ValueOf(td)
47+
}
48+
49+
func (Atom) Generate(rand *rand.Rand, size int) reflect.Value {
50+
a := Atom{}
51+
f := fuzz.New().RandSource(rand).MaxDepth(4)
52+
f.Fuzz(&a)
53+
return reflect.ValueOf(a)
54+
}
55+
2556
func TestEquals(t *testing.T) {
2657
// In general this test will make sure people update things when they
2758
// add a field.

‎typed/helpers.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,9 @@ func (ef errorFormatter) prefixError(prefix string, err error) ValidationErrors
9797
}
9898

9999
type atomHandler interface {
100-
doScalar(schema.Scalar) ValidationErrors
101-
doList(schema.List) ValidationErrors
102-
doMap(schema.Map) ValidationErrors
100+
doScalar(*schema.Scalar) ValidationErrors
101+
doList(*schema.List) ValidationErrors
102+
doMap(*schema.Map) ValidationErrors
103103

104104
errorf(msg string, args ...interface{}) ValidationErrors
105105
}
@@ -130,11 +130,11 @@ func deduceAtom(a schema.Atom, v *value.Value) schema.Atom {
130130
func handleAtom(a schema.Atom, tr schema.TypeRef, ah atomHandler) ValidationErrors {
131131
switch {
132132
case a.Map != nil:
133-
return ah.doMap(*a.Map)
133+
return ah.doMap(a.Map)
134134
case a.Scalar != nil:
135-
return ah.doScalar(*a.Scalar)
135+
return ah.doScalar(a.Scalar)
136136
case a.List != nil:
137-
return ah.doList(*a.List)
137+
return ah.doList(a.List)
138138
}
139139

140140
name := "inlined"
@@ -145,14 +145,14 @@ func handleAtom(a schema.Atom, tr schema.TypeRef, ah atomHandler) ValidationErro
145145
return ah.errorf("schema error: invalid atom: %v", name)
146146
}
147147

148-
func (ef errorFormatter) validateScalar(t schema.Scalar, v *value.Value, prefix string) (errs ValidationErrors) {
148+
func (ef errorFormatter) validateScalar(t *schema.Scalar, v *value.Value, prefix string) (errs ValidationErrors) {
149149
if v == nil {
150150
return nil
151151
}
152152
if v.Null {
153153
return nil
154154
}
155-
switch t {
155+
switch *t {
156156
case schema.Numeric:
157157
if v.FloatValue == nil && v.IntValue == nil {
158158
// TODO: should the schema separate int and float?
@@ -195,7 +195,7 @@ func mapValue(val value.Value) (*value.Map, error) {
195195
}
196196
}
197197

198-
func keyedAssociativeListItemToPathElement(list schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
198+
func keyedAssociativeListItemToPathElement(list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
199199
pe := fieldpath.PathElement{}
200200
if child.Null {
201201
// For now, the keys are required which means that null entries
@@ -221,7 +221,7 @@ func keyedAssociativeListItemToPathElement(list schema.List, index int, child va
221221
return pe, nil
222222
}
223223

224-
func setItemToPathElement(list schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
224+
func setItemToPathElement(list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
225225
pe := fieldpath.PathElement{}
226226
switch {
227227
case child.MapValue != nil:
@@ -241,7 +241,7 @@ func setItemToPathElement(list schema.List, index int, child value.Value) (field
241241
}
242242
}
243243

244-
func listItemToPathElement(list schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
244+
func listItemToPathElement(list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
245245
if list.ElementRelationship == schema.Associative {
246246
if len(list.Keys) > 0 {
247247
return keyedAssociativeListItemToPathElement(list, index, child)

‎typed/merge.go

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func (w *mergingWalker) doLeaf() {
104104
w.rule(w)
105105
}
106106

107-
func (w *mergingWalker) doScalar(t schema.Scalar) (errs ValidationErrors) {
107+
func (w *mergingWalker) doScalar(t *schema.Scalar) (errs ValidationErrors) {
108108
errs = append(errs, w.validateScalar(t, w.lhs, "lhs: ")...)
109109
errs = append(errs, w.validateScalar(t, w.rhs, "rhs: ")...)
110110
if len(errs) > 0 {
@@ -158,7 +158,7 @@ func (w *mergingWalker) derefMap(prefix string, v *value.Value, dest **value.Map
158158
return nil
159159
}
160160

161-
func (w *mergingWalker) visitListItems(t schema.List, lhs, rhs *value.List) (errs ValidationErrors) {
161+
func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs *value.List) (errs ValidationErrors) {
162162
out := &value.List{}
163163

164164
// TODO: ordering is totally wrong.
@@ -255,7 +255,7 @@ func (w *mergingWalker) derefList(prefix string, v *value.Value, dest **value.Li
255255
return nil
256256
}
257257

258-
func (w *mergingWalker) doList(t schema.List) (errs ValidationErrors) {
258+
func (w *mergingWalker) doList(t *schema.List) (errs ValidationErrors) {
259259
var lhs, rhs *value.List
260260
w.derefList("lhs: ", w.lhs, &lhs)
261261
w.derefList("rhs: ", w.rhs, &rhs)
@@ -280,23 +280,15 @@ func (w *mergingWalker) doList(t schema.List) (errs ValidationErrors) {
280280
return errs
281281
}
282282

283-
func (w *mergingWalker) visitMapItems(t schema.Map, lhs, rhs *value.Map) (errs ValidationErrors) {
283+
func (w *mergingWalker) visitMapItems(t *schema.Map, lhs, rhs *value.Map) (errs ValidationErrors) {
284284
out := &value.Map{}
285285

286-
fieldTypes := map[string]schema.TypeRef{}
287-
for i := range t.Fields {
288-
// I don't want to use the loop variable since a reference
289-
// might outlive the loop iteration (in an error message).
290-
f := t.Fields[i]
291-
fieldTypes[f.Name] = f.Type
292-
}
293-
294286
if lhs != nil {
295287
for i := range lhs.Items {
296288
litem := &lhs.Items[i]
297289
fieldType := t.ElementType
298-
if ft, ok := fieldTypes[litem.Name]; ok {
299-
fieldType = ft
290+
if sf, ok := t.FindField(litem.Name); ok {
291+
fieldType = sf.Type
300292
}
301293
w2 := w.prepareDescent(fieldpath.PathElement{FieldName: &litem.Name}, fieldType)
302294
w2.lhs = &litem.Value
@@ -324,8 +316,8 @@ func (w *mergingWalker) visitMapItems(t schema.Map, lhs, rhs *value.Map) (errs V
324316
}
325317

326318
fieldType := t.ElementType
327-
if ft, ok := fieldTypes[ritem.Name]; ok {
328-
fieldType = ft
319+
if sf, ok := t.FindField(ritem.Name); ok {
320+
fieldType = sf.Type
329321
}
330322
w2 := w.prepareDescent(fieldpath.PathElement{FieldName: &ritem.Name}, fieldType)
331323
w2.rhs = &ritem.Value
@@ -344,7 +336,7 @@ func (w *mergingWalker) visitMapItems(t schema.Map, lhs, rhs *value.Map) (errs V
344336
return errs
345337
}
346338

347-
func (w *mergingWalker) doMap(t schema.Map) (errs ValidationErrors) {
339+
func (w *mergingWalker) doMap(t *schema.Map) (errs ValidationErrors) {
348340
var lhs, rhs *value.Map
349341
w.derefMap("lhs: ", w.lhs, &lhs)
350342
w.derefMap("rhs: ", w.rhs, &rhs)

‎typed/remove.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ func removeItemsWithSchema(value *value.Value, toRemove *fieldpath.Set, schema *
3838
// will be a descent. It modifies w.inLeaf.
3939
func (w *removingWalker) doLeaf() ValidationErrors { return nil }
4040

41-
func (w *removingWalker) doScalar(t schema.Scalar) ValidationErrors { return nil }
41+
func (w *removingWalker) doScalar(t *schema.Scalar) ValidationErrors { return nil }
4242

43-
func (w *removingWalker) doList(t schema.List) (errs ValidationErrors) {
43+
func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) {
4444
l := w.value.ListValue
4545

4646
// If list is null, empty, or atomic just return
@@ -70,7 +70,7 @@ func (w *removingWalker) doList(t schema.List) (errs ValidationErrors) {
7070
return nil
7171
}
7272

73-
func (w *removingWalker) doMap(t schema.Map) ValidationErrors {
73+
func (w *removingWalker) doMap(t *schema.Map) ValidationErrors {
7474
m := w.value.MapValue
7575

7676
// If map is null, empty, or atomic just return

0 commit comments

Comments
 (0)