Improve CodeFormatter efficiency #272
Conversation
| context["successResponse"] = successResponses.first | ||
| context["successType"] = successResponse.flatMap(getResponseContext)?["type"] | ||
| context["defaultResponse"] = responses.first { $0.statusCode == nil }.flatMap(getResponseContext) | ||
| context["onlySuccessResponses"] = successResponse != nil && responses.count == 1 |
There was a problem hiding this comment.
In this PR I'm keeping the current behavior.
However, I wonder:
shouldn't this be context["onlySuccessResponses"] = successResponses.count == responses.count?
There was a problem hiding this comment.
Yep, that would make sense. The behaviour would be slightly different (this would be true with 2 success responses, where before it would be false), but the way it's used in the template, it would actually be more correct
There was a problem hiding this comment.
Would you like me to make this change?
I haven't dove much deeper into the library: I'm not sure what this change could cause.
There was a problem hiding this comment.
Yeah happy with it. It better reflects the name
There was a problem hiding this comment.
Done! I've also updated the CHANGELOG. I think this PR is ready to go! 🎉
There was a problem hiding this comment.
Actually this does seem to have broken something. You can see the result if you run swift test and see the generated diff
There was a problem hiding this comment.
I restored the previous behavior: now all tests pass!
|
Great, thanks @zntfdr. There are definitely many low hanging performance optimisations possible. |
Done 😊 |
|
Updated by also improving I will create new PRs if I spot further similar improvements. |
* remove unnecessary successResponse * avoid calling getResponseContext unnecessarily * update changelog * avoid calling getSchemaContext unnecessarily * allow onlySuccessResponses to be true when there are multiple success responses * restore previous behaviour * remove no longer true changelog
I've noticed that in
CodeFormatter'sgetOperationContext(_:)we can avoid callinggetResponseContext(_:)a few times.I'm working with a big
.yamlfile:In my testing this can save around 25% of memory use, and reduce
swaggen's execution by a few seconds as well.No change in the final output