-
-
Notifications
You must be signed in to change notification settings - Fork 490
Description
Hi, happy users of this library here.
TL;DR: Disabling yaml support would improve loading performance significantly for our use case. Can we make it possible?
During loading, invopop/yaml is used to unmarshal the yaml or json document. Internally, invopop/yaml first converts
the document to json, then unmarshals it using the standard library json package. Even if the document is already
in json format, the conversion step adds significant overhead [1].
In our (probably rather uncommon) use case, the specs we want to load are:
- always in json format
- rather large (up to 100mb)
Using only the standard library json package to unmarshal improves loading performance by multiple seconds for us. We have a functioning workaround to achieve this [2], but it feels brittle and hacky.
What's your opinion on adding an officially supported possibility to disable yaml support for improved performance? I have two potential approaches in mind:
- An explicit config parameter disabling yaml support
- Making the
Unmarshalfunction used by the loader a package variable or field on the loader struct so that users
can plug in their own implementation (which would bejson.Unmarshalin our case)
[1] - For a very basic benchmark, see https://gist.github.com/balogal/1119c62b46fec86719fcbdbd520e086c which compares the performance of json.Unmarshal (stdlib) and yaml.Unmarshal (invopop/yaml) for a small (7kb) and large (2.5mb) json workload. In both cases, the stdlib is around 6,5 times faster. Results for ghodss/yaml are comparable.
[2] - Workaround
func CustomLoadFromData(loader *openapi3.Loader, data []byte) (*openapi3.T, error) {
// loader.resetVisitedPathItemRefs() - loader needs to be fresh, we cannot reset visited refs because it's a internal function
doc := &openapi3.T{}
if err := json.Unmarshal(data, doc); err != nil {
return nil, err
}
if err := loader.ResolveRefsIn(doc, nil); err != nil {
return nil, err
}
return doc, nil
}