Skip to content

Commit

Permalink
perf(validators): reduced GC pressure
Browse files Browse the repository at this point in the history
This PR is a follow-up on go-openapi#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 (go-openapi#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 <fredbi@yahoo.com>
  • Loading branch information
fredbi committed Jan 27, 2024
1 parent 7c3e606 commit 44a8ced
Show file tree
Hide file tree
Showing 26 changed files with 7,163 additions and 186 deletions.
13 changes: 11 additions & 2 deletions BENCHMARK.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Validating the Kubernetes Swagger API

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

After refact PR: minor but noticable improvements: 25,000,000 allocs
## After refact PR: minor but noticable improvements: 25,000,000 allocs
```
go test -bench Spec
goos: linux
Expand All @@ -20,3 +20,12 @@ pkg: github.com/go-openapi/validate
cpu: AMD Ryzen 7 5800X 8-Core Processor
Benchmark_KubernetesSpec/validating_kubernetes_API-16 1 4064535557 ns/op 3379715592 B/op 25320330 allocs/op
```

## After reduce GC pressure PR: 17,000,000 allocs
```
goos: linux
goarch: amd64
pkg: github.com/go-openapi/validate
cpu: AMD Ryzen 7 5800X 8-Core Processor
Benchmark_KubernetesSpec/validating_kubernetes_API-16 1 3758414145 ns/op 2593881496 B/op 17111373 allocs/op
```
1 change: 1 addition & 0 deletions benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func Benchmark_KubernetesSpec(b *testing.B) {

for i := 0; i < b.N; i++ {
validator := NewSpecValidator(doc.Schema(), strfmt.Default)
validator.Options.SkipSchemataResult = true
res, _ := validator.Validate(doc)
if res == nil || !res.IsValid() {
b.FailNow()
Expand Down
45 changes: 20 additions & 25 deletions default_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,31 +50,25 @@ func isVisited(path string, visitedSchemas map[string]struct{}) bool {
}

// search for overlapping paths
frags := strings.Split(path, ".")
if len(frags) < 2 {
// shortcut exit on smaller paths
return found
}
last := len(frags) - 1
var currentFragStr, parent string
for i := range frags {
if i == 0 {
currentFragStr = frags[last]
} else {
currentFragStr = strings.Join([]string{frags[last-i], currentFragStr}, ".")
}
if i < last {
parent = strings.Join(frags[0:last-i], ".")
} else {
parent = ""
var (
parent string
suffix string
)
for i := len(path) - 2; i >= 0; i-- {
r := path[i]
if r != '.' {
continue
}
if strings.HasSuffix(parent, currentFragStr) {
found = true
break

parent = path[0:i]
suffix = path[i+1:]

if strings.HasSuffix(parent, suffix) {
return true
}
}

return found
return false
}

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

// Validate validates the default values declared in the swagger spec
func (d *defaultValidator) Validate() *Result {
errs := new(Result)
errs := poolOfResults.BorrowResult() // will redeem when merged

if d == nil || d.SpecValidator == nil {
return errs
}
Expand All @@ -102,7 +97,7 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result {
// every default value that is specified must validate against the schema for that property
// headers, items, parameters, schema

res := new(Result)
res := poolOfResults.BorrowResult() // will redeem when merged
s := d.SpecValidator

for method, pathItem := range s.expandedAnalyzer().Operations() {
Expand Down Expand Up @@ -232,7 +227,7 @@ func (d *defaultValidator) validateDefaultValueSchemaAgainstSchema(path, in stri
return nil
}
d.beingVisited(path)
res := new(Result)
res := poolOfResults.BorrowResult()
s := d.SpecValidator

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

func (d *defaultValidator) validateDefaultValueItemsAgainstSchema(path, in string, root interface{}, items *spec.Items) *Result {
res := new(Result)
res := poolOfResults.BorrowResult()
s := d.SpecValidator
if items != nil {
if items.Default != nil {
Expand Down
15 changes: 10 additions & 5 deletions example_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ func (ex *exampleValidator) isVisited(path string) bool {
// - individual property
// - responses
func (ex *exampleValidator) Validate() *Result {
errs := new(Result)
errs := poolOfResults.BorrowResult()

if ex == nil || ex.SpecValidator == nil {
return errs
}
Expand All @@ -74,7 +75,7 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result {
// in: schemas, properties, object, items
// not in: headers, parameters without schema

res := new(Result)
res := poolOfResults.BorrowResult()
s := ex.SpecValidator

for method, pathItem := range s.expandedAnalyzer().Operations() {
Expand Down Expand Up @@ -105,6 +106,8 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result {
if red.HasErrorsOrWarnings() {
res.AddWarnings(exampleValueItemsDoesNotValidateMsg(param.Name, param.In))
res.Merge(red)
} else {
poolOfResults.RedeemResult(red)
}
}

Expand All @@ -114,6 +117,8 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result {
if red.HasErrorsOrWarnings() {
res.AddWarnings(exampleValueDoesNotValidateMsg(param.Name, param.In))
res.Merge(red)
} else {
poolOfResults.RedeemResult(red)
}
}
}
Expand Down Expand Up @@ -220,7 +225,7 @@ func (ex *exampleValidator) validateExampleValueSchemaAgainstSchema(path, in str
}
ex.beingVisited(path)
s := ex.SpecValidator
res := new(Result)
res := poolOfResults.BorrowResult()

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

func (ex *exampleValidator) validateExampleValueItemsAgainstSchema(path, in string, root interface{}, items *spec.Items) *Result {
res := new(Result)
res := poolOfResults.BorrowResult()
s := ex.SpecValidator
if items != nil {
if items.Example != nil {
Expand All @@ -275,7 +280,7 @@ func (ex *exampleValidator) validateExampleValueItemsAgainstSchema(path, in stri
)
}
if items.Items != nil {
res.Merge(ex.validateExampleValueItemsAgainstSchema(path+"[0].example", in, root, items.Items)) // TODO(fred): intern string
res.Merge(ex.validateExampleValueItemsAgainstSchema(path+"[0].example", in, root, items.Items))
}
if _, err := compileRegexp(items.Pattern); err != nil {
res.AddErrors(invalidPatternInMsg(path, in, items.Pattern))
Expand Down

0 comments on commit 44a8ced

Please sign in to comment.