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

apply plugin: 'de.thetaphi.forbiddenapis' is very slow. #116

Closed
huxi opened this issue Jan 31, 2017 · 19 comments
Closed

apply plugin: 'de.thetaphi.forbiddenapis' is very slow. #116

huxi opened this issue Jan 31, 2017 · 19 comments
Assignees
Milestone

Comments

@huxi
Copy link

huxi commented Jan 31, 2017

allprojects {
    apply plugin: 'de.thetaphi.forbiddenapis'
}

adds 25s to the configuration time of a multi-module build with about 500 modules.

The reason for this is that ForbiddenApisPlugin.java is compiling the plugin-init.groovy script for every single apply call.

You should instead re-implement the plugin-init.groovy functionality in Java or switch the ForbiddenApisPlugin to Groovy.

@uschindler
Copy link
Member

For several reasons I will not add precompiled Groovy code at the moment (not compatible to the setup of compiling the plugin for 3 build systems + CLI in one single artifact).

The only idea would be to only parse the groovy script once and reuse the compiled class. That should be easy to implement.

@uschindler uschindler self-assigned this Jan 31, 2017
@uschindler uschindler added this to the 2.3 milestone Jan 31, 2017
uschindler added a commit that referenced this issue Jan 31, 2017
@uschindler
Copy link
Member

Hi,
can you quickly compile the branch with this commit: 1fc0a16
I don't have such a large project at hand (and I have not much time at the moment), so I cannot test this (if it helps).

The reason why I am not sure is: It is undefined in the Gradle docs, if a new instance of the plugin is created for all subprojects, or if there is a single instance and the apply() method is just called multiple times.

If you have some time to figure that out, I would be happy.

@uschindler
Copy link
Member

An alternative would be to init the script in the static initializer, but if there is only a single instance of the Plugin, I would prefer to have it like that.

FYI, creating the plugin-init code in pure Java is not really doable for several reasons:

  • there is no complete Gradle on Maven Central, so you have nothing you could compile against.
  • It uses Groovy language constructs (for the extensions), that are not easy to port.

@uschindler
Copy link
Member

Oh oh, I think I should compile the script on clinit: http://stackoverflow.com/questions/27026996/what-is-gradle-plugin-lifecycle

Will work on this later!

@uschindler
Copy link
Member

I pushed a new version that "compiles" the script on loading the plugin class.

@uschindler
Copy link
Member

See the PR #117

@huxi
Copy link
Author

huxi commented Jan 31, 2017

268ms configuration time with the 500+ module Gradle build using #117. 👍

FYI: I had to manually create build/maven/generated-site and build/maven/generated-site/xdoc. Otherwise the Ant build wouldn't work for me.

@oehme
Copy link

oehme commented Jan 31, 2017

May I ask why it needs to be Groovy at all? Why can't that piece of code not be written in Java like the rest of the plugin?

@uschindler
Copy link
Member

@oehme the problem was mentioned before: The ForbiddenAPIs plugin supports Ant, Maven, Gradle and a CLI interface. Yes you could theoretically write everything in Java, but there are some problems:

  • There is no complete Gradle on Maven Central. You can only build a full-featured Gradle plugin using Gradle and have to use the special gradleAll() dependency do get a full classpath (which adds the local Gradle to classpath, so it don't need to be on Maven Central or jCenter). But this is broken, because it pollutes your classpath with old ASM versions and other stuff, incompatible to forbiddenapis.
  • This projects builds with Ant + Ivy (for several reasons, including the previous one): I tried to port it to build with Gradle, but then got stuck on the previous point. As far as I know: They fixed the classpath pollution partially, so this might change at some point. But this disallows to test Java 9, because Gradle is heavy broken at the moment.
  • The Gradle API is on a Maven Repository so you can use it, but all "internal" stuff like the Java plugin is missing. So the plugin initializes itsself with dynamic scripting!

@uschindler
Copy link
Member

uschindler commented Jan 31, 2017

FYI: I had to manually create build/maven/generated-site and build/maven/generated-site/xdoc. Otherwise the Ant build wouldn't work for me.

You should be able to run ant clean dist and it should install itsself in the local Maven Repository. How did you call Ant? And what was the error message? Maybe some version conflict, but I have no idea how this could happen!

@uschindler
Copy link
Member

268ms configuration time with the 500+ module Gradle build using #117. 👍

Thanks, so this PR fixes the issue. I will merge that later!

@huxi
Copy link
Author

huxi commented Jan 31, 2017

I simply executed ant.
Oh, and I'm on a Mac so that might be part of the reason.

@oehme
Copy link

oehme commented Jan 31, 2017

The Gradle API is on a Maven Repository so you can use it, but all "internal" stuff like the Java plugin is missing. So the plugin initializes itsself with dynamic scripting!

Thanks for the explanation. We are planning to make that easier in the future, as we've seen some other projects as well who are stuck on Maven, but still want to provide a Gradle plugin.

@uschindler
Copy link
Member

@huxi OK. Id like to fix this. Could you create another issue and post the whole build output there?
Executing "ant" alone is the same as "ant dist", so there should be no difference. And I tried it locally, it worked. Maybe it is really a Mac issue.

Maybe I should explicitely create the missing directories in the ant script.

@uschindler
Copy link
Member

@oehme Thanks! I hope this will be possible also with older, still Java-6-based Gradle versions, so the forbiddenapis plugin can stay compatible with Java 6 (it is currently built against Java 6). If you would publish Maven artifacts only for newer versions, this may still not solve the issue (unless forbiddenapis requires Java 7 minimum at some point). Keep me updated! :-)

@huxi
Copy link
Author

huxi commented Jan 31, 2017

@uschindler Just tried ant clean followed by ant and it just worked. I even removed .ivy manually to really have a clean git state but to no avail.
I don't get it. Sorry, can't reproduce it anymore. Very strange.

uschindler added a commit that referenced this issue Jan 31, 2017
…ch invocation instead of an instance (according to Groovy guidelines)
@uschindler
Copy link
Member

@huxi No problem, sometimes strange things happens.

I added a small rewrite according to the Groovy guidelines. This new implementation now still compiles the script on static initializer, but actually to a Java class that is then reused to create several instances. This makes it thread safe, so I was able to remove the stupid synchronization.

@uschindler
Copy link
Member

The new implementation is now committed and pushed. Have fun! I try to release a new version soon, because other people were already complaining about some slowness for large projects, maybe that was related.

@huxi
Copy link
Author

huxi commented Feb 1, 2017

Thanks for taking care about this issue in such a timely fashion.

centic9 pushed a commit to centic9/forbidden-apis that referenced this issue Jan 16, 2018
…in class (policeman-tools#117)

Compile the Groovy script only once when loading the Gradle plugin class.
This closes policeman-tools#116
centic9 pushed a commit to centic9/forbidden-apis that referenced this issue Jan 16, 2018
…s reused for each invocation instead of an instance (according to Groovy guidelines)
centic9 pushed a commit to centic9/forbidden-apis that referenced this issue Jan 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants