-
Notifications
You must be signed in to change notification settings - Fork 455
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
Use task configuration avoidance internally (Spotless already supports externally) #544
Comments
Hi @dcabasson! This is a duplicate of #269, but I wouldn't blame you for not finding it; it was closed because although I opened a PR for it a long time ago, I never found the time and motivation to finish things off, and IIRC the issue was closed due to the lack of any promising speed improvement. If you felt up to picking things up where I left off, then feel free to copy the code in my old PR and to create a new one. :) @nedtwigg would be the best person to advise if the old issue should be reopened or not. |
Ah indeed. I went through the open tickets but I didn't find one which is why I created that one. I will work on the issue and propose a fix, time permitting. It's more of a clean up exercise to ensure you are using the current recommended Gradle approach. Indeed the speed gain will be modest, but every little bit helps (or hurts depending on the point of view) |
As of Spotless has had internal config avoidance even before Gradle provided support for that. When you specify a config file to Spotless, Spotless does not read or parse it until/unless it is needed. I am very skeptical that there will be any performance benefit to adopting Gradle config avoidance, although I am all for adopting any Gradle feature if we can do so without bumping requirements. Right now, we support all the way back to gradle I think it is good for us to break back-compat iff:
Because of how Spotless is implemented, the perf gains to adopting config avoidance are too small to measure. The only way we could implement it without bumping requirements would be to have two implementations (as we did #463), which is needlessly complicated if we're not getting anything useful in return. I'm leaving this open, but tagging it with wontfix. If something else happens which causes us to bump our minimum required gradle, then I'll happily merge this change. But I'm not going to bump our minimum requirements from |
@nedtwigg Wow, I'd completely forgotten about that part of the whole story, so thank you for the thorough reminder and explanation. 👍 |
As a further data point, I just worked with a Gradle Enterprise customer that has a very large, very well optimized build. While I couldn't extract the actual impact on configuration time of the Spotless plugin, I did observe that out of 10,000 tasks being eagerly created by the build, 7000 of them were created by the Spotless plugin! This was a multi-project build with around 500 subprojects. With 4 source sets configured for checks, Spotless eagerly creates 14 tasks per subproject. I would suggest:
|
Thanks for the datapoint! Here's my proposed roadmap. |
See #601 |
See https://docs.gradle.org/current/userguide/task_configuration_avoidance.html for more context
Currently the spotless gradle plugin is eagerly creating tasks, which is not the recommended approach anymore.
I think for the most part, the change should be https://github.com/diffplug/spotless/blob/master/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java#L631 to use
register
rather thancreate
and deal with the fact that this will be aTaskProvider
rather than aTask
.The text was updated successfully, but these errors were encountered: