Skip to content
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

Improve Gradle Configuration Time #428

Open
rwinch opened this issue May 18, 2018 · 12 comments
Open

Improve Gradle Configuration Time #428

rwinch opened this issue May 18, 2018 · 12 comments

Comments

@rwinch
Copy link

rwinch commented May 18, 2018

As a part of gradle/gradle#1628 I submitted Spring Security's Gradle Configuration Time.

The Gradle team replied with:

The vast majority of the time (75%) is spent in the Gretty plugin, which is doing a lot of heavy lifting in dynamic Groovy. If it was migrated to @CompileStatic (or Java or Kotlin) you’d see a big improvement in your configuration time.

It would be nice if the Gretty plugin could improve the configuration time by switching to @CompileStatic, Java, or Kotlin.

@wolfs
Copy link

wolfs commented May 18, 2018

See also gretty-gradle-plugin#46 for the same issue on the gretty fork.

@rwinch
Copy link
Author

rwinch commented May 18, 2018

@wolfs Thanks! I was not aware of the fork.

@javabrett
Copy link

javabrett commented May 31, 2018

@rwinch @wolfs So this issue has achieved something good and got me to learn to run the gradle-profiler, nice. I ran this for myself a couple of times on spring-security.

Being new to the analysis and with reference to https://scans.gradle.com/s/7yjns4i55vj66/ ... I was not able to see where it was inferred that Gretty consumed 75% of the configuration time. Is there a 10 second explainer for that, which numbers to read? In fact I had trouble working-out how to get the breakdown numbers to add to the totals ... are those timings averages per execution (that doesn't tally either).

@rwinch
Copy link
Author

rwinch commented Jun 6, 2018

@javabrett I didn't look at the results myself. I was told by the Gradle team after submitting my results. I'll see if they are able to help out here.

@oehme
Copy link

oehme commented Jun 7, 2018

@javabrett Please see this issue for instructions and make sure you update your Gradle profiler, I just merged a PR that produces easy to read flame graphs. You'll see Gretty consuming a huge amount of time in dynamic invocations in the CPU flame graph.

In general plugins should not contain any dynamic code unless absolutely necessary. I.e. the reasoning for closing the referenced PR was the wrong way around: You should always use @CompileStatic everywhere (or write Java or Kotlin), not just when someone explicitly told you about a performance problem.

For your convenience, here is a flame graph showing the overhead of Gretty. It's part of a bigger graph, but makes up about 75% of the CPU samples.
screen shot 2018-06-07 at 11 22 45

javabrett added a commit to gretty-gradle-plugin/gretty that referenced this issue Jun 9, 2018
This deactivates dynamic Groovy and forces static type-checking at compile-time. For
Gretty/Gradle this can significantly improve execution-performance during configuration
phase, as demonstrated by timing the execution of the simple "gradle help".

Fixes #46 (akhikhl#428).
@javabrett
Copy link

The configuration-time performance improvement from adding @CompileStatic should now be available in Gretty 2.3.0-SNAPSHOT and 3.0.0-SNAPSHOT. A 2.3.0 release shouldn't be too far away.

A subsequent buildscan for spring-security is available at https://scans.gradle.com/s/o6jlskwqa7k3y .

Thanks @rwinch for the report and @wolfs and @oehme for the profiler support.

@oehme
Copy link

oehme commented Jun 10, 2018

What an excellent result, going from 3s to 100ms. Thank you!

@wolfs
Copy link

wolfs commented Jun 11, 2018

@javabrett Did you post the right build scan? The scan you posted is for the gradle-profiler project, not for the spring security project. What is the improvement for the spring security project?

@javabrett
Copy link

javabrett commented Jun 11, 2018

Thanks @wolfs . Hmmm that's really annoying ... I guess I did not specify --project-dir . and reading https://github.com/gradle/gradle-profiler/blob/master/src/main/java/org/gradle/profiler/CommandLineParser.java#L22 :

defaultsTo(new File(".")

... I didn't spot that when invoking e.g. /path/to/gradle-profiler/build/install/gradle-profiler/bin/gradle-profiler ... help with cwd of the project-of-interest-to-profile, that's going to run for gradle-profiler. Intentional do you think?

Edit: Or maybe having just rebuilt gradle-profiler on master I was not paying attention and ran it in the wrong window totally screwing-up. Unlikely but very possible.

... which I guess is going to be nowhere near the improvement, if any. Does the buildscan profile provide any details, or is jfr with the flame graphs or something else needed to get the required details?

Since there's little or no improvement, I'll profile some more.

@oehme
Copy link

oehme commented Jun 11, 2018

For algorithmic problems like this you'll need to use JFR/Flamegraphs. They'll point you right to the code that is causing the problem. For instance you can clearly see in the graph I posted above that the FarmConfig is a big problem. This is one of the classes you explicitly excluded from static type checking. Overall the number of classes with TypeCheckMode.SKIP is still too high.

@javabrett
Copy link

javabrett commented Jun 11, 2018

A couple of testing updates and observations:

That said, I can now see some suspect code in Gretty (highlighted in the flame graphs) which is leaking test-configuration with production, and this seems to be the main cause of the performance problem. So while @CompileStatic might help, I think the key fix will be stop that code from executing.

@oehme
Copy link

oehme commented Jun 27, 2018

Could you elaborate what you mean by leaking test configuration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants