search

guardrail-dev / guardrail

Principled code generation from OpenAPI specifications
https://guardrail.dev
MIT License
523 stars 132 forks source link

Performance issues #496

Open BBartosz opened 4 years ago

BBartosz commented 4 years ago

What has changed between 0.54.1 and 0.54.5 that for pretty big swagger version 0.54.1 takes 79 seconds to generate code and version 0.54.5 takes 20 minutes/never terminates/hangs?

blast-hardcheese commented 4 years ago

@BBartosz Almost certainly related to #489.

I noticed the slowdown in our integration tests, but unfortunately not until it had already been released, I'm sorry this has impacted you.

338 was merged in 0.54.4, I expect to have 0.55.0 released sometime this weekend or early next week, which should resolve things.

If you have the time to test your specification against the current HEAD using ./cli.sh, that would be very much appreciated.

There are still some performance tweaks that can be done around the logger, so if the performance is still bad I can spend some more time optimizing that.

BBartosz commented 4 years ago

@blast-hardcheese wow, that would be great if you could release it this weekend.

blast-hardcheese commented 4 years ago

@BBartosz I'll see what I can do 😄 Is there something you're looking for that is in 0.54.5, or just trying to keep up to date? The reason for the big version bump is that it took some attempts to get Travis working again after the sbt-pgp -> sbt-gpg jump, as well as the sbt-sonatype update.

If you'd be able to provide some stats around running the current master branch against your large OpenAPI specification, I can't stress enough how much that would help. Thank you!

BBartosz commented 4 years ago

@blast-hardcheese right now we have to use 0.54.1 because 0.54.5 hangs and cannot generate swagger at all. I guess thats because my swagger has grown more. 2 weeks ago with slighly smaller swagger i was able to generate with 0.54.5 but it took ~20 minutes. So I would like to have newest version but just working. Moreover I have found one more bug when generating models in scala. Brief description: scala pics up wrong classes when you have model which looks like this:

class Passenger(passenger: Passenger)

So would like to have latest working version so I can play around. For now since 0.55 is not released i can publish local current master and try it, but if you will be able to release that would be great because i could put new version in codebase.

blast-hardcheese commented 4 years ago

@BBartosz Ah, sorry for the miscommunication! I mean to clone the guardrail repo and use https://github.com/twilio/guardrail/blob/master/cli.sh, the guardrail CLI, to verify that the processing time becomes reasonable.

That would help me know whether the fix I merged is sufficient or not.

Thank you

blast-hardcheese commented 4 years ago

OK, 0.55.0 has been released. If you get a chance to try it with your very large specification and can report how long it takes, that would help a lot. Thank you for your patience!

BBartosz commented 4 years ago

@blast-hardcheese awesome, was busy on weekend, but trying it now.

blast-hardcheese commented 4 years ago

@BBartosz Any luck?

BBartosz commented 4 years ago

@blast-hardcheese No luck. Still hangs during generation. 0.54.1 works correctly. Edit. Actually it finally finished after 20mins. But still 0.54.1 finished in 2 :/

blast-hardcheese commented 4 years ago

that's really unacceptable, I'm sorry to hear it. Let me see if I can just turn that functionality off for the time being, until there's a way to implement it that doesn't drastically negatively impact performance.

blast-hardcheese commented 4 years ago

@BBartosz Would you be able to try your specification against #508? You can use ./cli.sh in order to test against a branch without waiting for a release

blast-hardcheese commented 4 years ago

@BBartosz If the code in #508 generates in a reasonable amount of time, would you mind comparing the amount of time taken against https://github.com/blast-hardcheese/guardrail/tree/reintroduce-logger as well?

Alternately, I could do this myself if you are able to either share the specification, a redacted form of the specification, or parameters I could use to craft a similar specification

BBartosz commented 4 years ago

@blast-hardcheese hey sorry i was on vacation, will try it today.