Skip to content

Commit 8c60715

Browse files
committed
perf(validators): reduced GC pressure
This PR is a follow-up on #169, which cames with innocuous changes, but already reduced the total number of allocations when validating a swagger spec. We reduce further the pressure on go's GC by pooling temporarily allocated objects: validators and results. When recursively allocating validators, calls to "Validate()" redeem to the pool the allocated validator. This means that a validator is usable only once. A similar approach is taken for `Result`s: upon merge, the merged result is redeemed to the pool. Eventually, temporarily allocated `spec.Schema`s are also pooled. In order not to break existing interfaces, this is enabled by default only when validating a spec (`Spec()`) or when using `AgainstSchema()`. Another option at the spec level allows for skipping the gathering of schemas into the result ("schemata"): these schemas must be cloned to remain valid after their parent is redeemed to the pool. Comment on the benchmark: CPU profiling shows that about 30% of the CPU time is spent managing garbage. Memory profiling shows that our benchmark (validating the k8s API spec) goes down from 25M allocated objects down to ~ 17M (#169 went down from 60M to 25M). The `Validate` method generates only ~ 5M of those, out of which ~ 4M are caused by the schema unmarshaling & expansion and 1M by string allocations for the validator "path". Unfortunately, classic string interning methods don't work well with this dynamic "path", so I leave it there. Integration tests (go-swagger/go-swagger#3064) show a significant improvement when running go-swagger validate in codegen. * contributes: go-swagger/go-swagger#2649 Further improvements would most likely come from optimizing the validation of defaults and examples (which amount for 2M allocs). Signed-off-by: Frederic BIDON <[email protected]>
1 parent 7c3e606 commit 8c60715

26 files changed

+7163
-186
lines changed

BENCHMARK.md

+11-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
Validating the Kubernetes Swagger API
44

5-
v0.22.6: 60,000,000 allocs
5+
## v0.22.6: 60,000,000 allocs
66
```
77
goos: linux
88
goarch: amd64
@@ -11,7 +11,7 @@ cpu: AMD Ryzen 7 5800X 8-Core Processor
1111
Benchmark_KubernetesSpec/validating_kubernetes_API-16 1 8549863982 ns/op 7067424936 B/op 59583275 allocs/op
1212
```
1313

14-
After refact PR: minor but noticable improvements: 25,000,000 allocs
14+
## After refact PR: minor but noticable improvements: 25,000,000 allocs
1515
```
1616
go test -bench Spec
1717
goos: linux
@@ -20,3 +20,12 @@ pkg: github.com/go-openapi/validate
2020
cpu: AMD Ryzen 7 5800X 8-Core Processor
2121
Benchmark_KubernetesSpec/validating_kubernetes_API-16 1 4064535557 ns/op 3379715592 B/op 25320330 allocs/op
2222
```
23+
24+
## After reduce GC pressure PR: 17,000,000 allocs
25+
```
26+
goos: linux
27+
goarch: amd64
28+
pkg: github.com/go-openapi/validate
29+
cpu: AMD Ryzen 7 5800X 8-Core Processor
30+
Benchmark_KubernetesSpec/validating_kubernetes_API-16 1 3758414145 ns/op 2593881496 B/op 17111373 allocs/op
31+
```

benchmark_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ func Benchmark_KubernetesSpec(b *testing.B) {
2121

2222
for i := 0; i < b.N; i++ {
2323
validator := NewSpecValidator(doc.Schema(), strfmt.Default)
24+
validator.Options.SkipSchemataResult = true
2425
res, _ := validator.Validate(doc)
2526
if res == nil || !res.IsValid() {
2627
b.FailNow()

default_validator.go

+20-25
Original file line numberDiff line numberDiff line change
@@ -50,31 +50,25 @@ func isVisited(path string, visitedSchemas map[string]struct{}) bool {
5050
}
5151

5252
// search for overlapping paths
53-
frags := strings.Split(path, ".")
54-
if len(frags) < 2 {
55-
// shortcut exit on smaller paths
56-
return found
57-
}
58-
last := len(frags) - 1
59-
var currentFragStr, parent string
60-
for i := range frags {
61-
if i == 0 {
62-
currentFragStr = frags[last]
63-
} else {
64-
currentFragStr = strings.Join([]string{frags[last-i], currentFragStr}, ".")
65-
}
66-
if i < last {
67-
parent = strings.Join(frags[0:last-i], ".")
68-
} else {
69-
parent = ""
53+
var (
54+
parent string
55+
suffix string
56+
)
57+
for i := len(path) - 2; i >= 0; i-- {
58+
r := path[i]
59+
if r != '.' {
60+
continue
7061
}
71-
if strings.HasSuffix(parent, currentFragStr) {
72-
found = true
73-
break
62+
63+
parent = path[0:i]
64+
suffix = path[i+1:]
65+
66+
if strings.HasSuffix(parent, suffix) {
67+
return true
7468
}
7569
}
7670

77-
return found
71+
return false
7872
}
7973

8074
// beingVisited asserts a schema is being visited
@@ -89,7 +83,8 @@ func (d *defaultValidator) isVisited(path string) bool {
8983

9084
// Validate validates the default values declared in the swagger spec
9185
func (d *defaultValidator) Validate() *Result {
92-
errs := new(Result)
86+
errs := poolOfResults.BorrowResult() // will redeem when merged
87+
9388
if d == nil || d.SpecValidator == nil {
9489
return errs
9590
}
@@ -102,7 +97,7 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result {
10297
// every default value that is specified must validate against the schema for that property
10398
// headers, items, parameters, schema
10499

105-
res := new(Result)
100+
res := poolOfResults.BorrowResult() // will redeem when merged
106101
s := d.SpecValidator
107102

108103
for method, pathItem := range s.expandedAnalyzer().Operations() {
@@ -232,7 +227,7 @@ func (d *defaultValidator) validateDefaultValueSchemaAgainstSchema(path, in stri
232227
return nil
233228
}
234229
d.beingVisited(path)
235-
res := new(Result)
230+
res := poolOfResults.BorrowResult()
236231
s := d.SpecValidator
237232

238233
if schema.Default != nil {
@@ -278,7 +273,7 @@ func (d *defaultValidator) validateDefaultValueSchemaAgainstSchema(path, in stri
278273
// TODO: Temporary duplicated code. Need to refactor with examples
279274

280275
func (d *defaultValidator) validateDefaultValueItemsAgainstSchema(path, in string, root interface{}, items *spec.Items) *Result {
281-
res := new(Result)
276+
res := poolOfResults.BorrowResult()
282277
s := d.SpecValidator
283278
if items != nil {
284279
if items.Default != nil {

example_validator.go

+10-5
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ func (ex *exampleValidator) isVisited(path string) bool {
5959
// - individual property
6060
// - responses
6161
func (ex *exampleValidator) Validate() *Result {
62-
errs := new(Result)
62+
errs := poolOfResults.BorrowResult()
63+
6364
if ex == nil || ex.SpecValidator == nil {
6465
return errs
6566
}
@@ -74,7 +75,7 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result {
7475
// in: schemas, properties, object, items
7576
// not in: headers, parameters without schema
7677

77-
res := new(Result)
78+
res := poolOfResults.BorrowResult()
7879
s := ex.SpecValidator
7980

8081
for method, pathItem := range s.expandedAnalyzer().Operations() {
@@ -105,6 +106,8 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result {
105106
if red.HasErrorsOrWarnings() {
106107
res.AddWarnings(exampleValueItemsDoesNotValidateMsg(param.Name, param.In))
107108
res.Merge(red)
109+
} else {
110+
poolOfResults.RedeemResult(red)
108111
}
109112
}
110113

@@ -114,6 +117,8 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result {
114117
if red.HasErrorsOrWarnings() {
115118
res.AddWarnings(exampleValueDoesNotValidateMsg(param.Name, param.In))
116119
res.Merge(red)
120+
} else {
121+
poolOfResults.RedeemResult(red)
117122
}
118123
}
119124
}
@@ -220,7 +225,7 @@ func (ex *exampleValidator) validateExampleValueSchemaAgainstSchema(path, in str
220225
}
221226
ex.beingVisited(path)
222227
s := ex.SpecValidator
223-
res := new(Result)
228+
res := poolOfResults.BorrowResult()
224229

225230
if schema.Example != nil {
226231
res.MergeAsWarnings(
@@ -266,7 +271,7 @@ func (ex *exampleValidator) validateExampleValueSchemaAgainstSchema(path, in str
266271
//
267272

268273
func (ex *exampleValidator) validateExampleValueItemsAgainstSchema(path, in string, root interface{}, items *spec.Items) *Result {
269-
res := new(Result)
274+
res := poolOfResults.BorrowResult()
270275
s := ex.SpecValidator
271276
if items != nil {
272277
if items.Example != nil {
@@ -275,7 +280,7 @@ func (ex *exampleValidator) validateExampleValueItemsAgainstSchema(path, in stri
275280
)
276281
}
277282
if items.Items != nil {
278-
res.Merge(ex.validateExampleValueItemsAgainstSchema(path+"[0].example", in, root, items.Items)) // TODO(fred): intern string
283+
res.Merge(ex.validateExampleValueItemsAgainstSchema(path+"[0].example", in, root, items.Items))
279284
}
280285
if _, err := compileRegexp(items.Pattern); err != nil {
281286
res.AddErrors(invalidPatternInMsg(path, in, items.Pattern))

0 commit comments

Comments
 (0)