-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
[CSharp-Netcore][Go] Optimize time taken to generate CSharp code #10152
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
Conversation
Merge upstream fixes
update with upstream master
…tor into feature/refresh_master
regular merge code from upstream
|
/cc @wing328 @mandrean @frankyjuang @shibayan @Blackclaws @lucamazzanti |
|
Just out of curiosity if you're seeing generation times of 2 hours how large is your api definition file? I've been running the generator on what I would consider intermediate sized apis and it never took more than a minute. |
modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
Outdated
Show resolved
Hide resolved
...penapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractCSharpCodegen.java
Outdated
Show resolved
Hide resolved
Our spec is very large, over 350k lines, 5000+ model objects. You can check it out at https://cdn.intersight.com/components/an-apidocs/1.0.9-4430/model/intersight-openapi-v3-1.0.9.4430.yaml |
|
Ok, yeah that is indeed a spec size that not a lot of people will have. That being said I have no issue with this pull request and it looks good to me, performance enhancements are always welcome even if they are mostly relevant to edge cases :) |
|
I'm wondering if this seems to be an issue in multiple generators, whether this caching shouldn't be going into the default codegen somehow |
so my initial fix was that - 8452565 But after some pointers from @wing328 it was clear that toModelName() is also invoked at many other places and so a better fix was to memoize toModelName itself. |
|
If it took two hours before, how long does it take after this change? |
Good point. I'll see what I can do to address the problem in the default codegen. |
Down to a few minutes if I'm not mistaken. |
1h56m -> 4m |
|
LGTM. I may submit another PR later to try to address similar problem for all generators. |
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.For Windows users, please run the script in Git BASH.
master,5.3.x,6.0.xThis PR greatly reduces the time taken for Csharp-netcore code generation.
toModelName() method is invoked numerous number of times and it was accummulating to a lot of time.
This PR adds memoization to CSharp and DefaultCodeGen toModelName() methods.
This has reduced the C# SDK generation time for our rather large openapi spec from ~2 hours to 4 mins.
This has reduced the Golang SDK generation time for our rather large openapi spec from ~1h30m hours to 4 mins.
The generated code is the same as before.
spec: https://cdn.intersight.com/components/an-apidocs/1.0.9-4430/model/intersight-openapi-v3-1.0.9.4430.yaml