-
Notifications
You must be signed in to change notification settings - Fork 210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize GetOpenAPIDefinitions to avoid generating the map for all definitions on every BuildOpenAPISpec invocation #299
Conversation
@@ -176,12 +185,13 @@ func ConvertConfigToV3(config *Config) *OpenAPIV3Config { | |||
GetOperationIDAndTags: config.GetOperationIDAndTags, | |||
GetOperationIDAndTagsFromRoute: config.GetOperationIDAndTagsFromRoute, | |||
GetDefinitionName: config.GetDefinitionName, | |||
Definitions: config.Definitions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must be mis-reading, but what sets that definitions? I can only see it read, I can't find where it's written.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k/k config setup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah, right of course
How much of an improvement is this? |
Reduces memory allocs by the GetDefinitions function from ~5mb x 60 group versions = ~300mb to 5mb (shared struct across all gvs). In use memory is unaffected after the openapi is built. |
That's super sweet, thanks! |
LGTM for the second commit. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: apelisse, Jefftree The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Optimize GetOpenAPIDefinitions to avoid generating the map for all definitions on every BuildOpenAPISpec invocation.
Use a pre-existing generated Definitions object if possible to avoid going through the generator process to generate the map of all Definitions.
Significantly reduces memory allocation footprint.
Please only look at latest commit. Branches off of #292
This PR only optimizes for V3, but we can apply the optimization later for V2 as well.
/assign @apelisse