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

Running spotless seems to do a full build #75

Open
badsyntax opened this issue Jun 18, 2020 · 10 comments
Open

Running spotless seems to do a full build #75

badsyntax opened this issue Jun 18, 2020 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@badsyntax
Copy link
Owner

This is evident on slow machines. When running spotless you see in the statusbar "Configure project" etc.

@badsyntax badsyntax added the enhancement New feature or request label Jun 18, 2020
@badsyntax badsyntax self-assigned this Jun 18, 2020
@nedtwigg
Copy link

nedtwigg commented Aug 25, 2020

It is a full build in that spotlessApply -PspotlessIdeHook will have to configure some tasks in the gradle project. This has gotten much faster in recent Gradle releases thanks to configuration-avoidance becoming more widespread. If the project makes full use of config-avoidance, it should be close to instantaneous. Spotless does a better job of this post 5.0 (released 2020-07-12).

One speedup you could try for multi-module projects is:

  • instead of calling spotlessApply
  • look up the directory tree until you find subproject/build.gradle
  • now call :subproject:spotlessApply :spotlessApply -PspotlessIdeblahblahblah
    • or possibly just :spotlessApply -PspoltessIdeblah if the file happens to not be in a subproject

By specifying that you only want to run the subproject spotlessApply and its parent(s), this allows Gradle to skip configuration of every other subproject. There's no way for Spotless to do that optimization internally unfortunately - we don't get called until Gradle has finished configuring, so only you can limit the scope of that configuration. EDIT: we can and should do this inside Spotless, see comment below.

Another avenue of speed up is the new "configuration cache" introduced in 6.6, which will make things much faster yet again. Here's the tracking issue for supporting that in Spotless diffplug/spotless#644 I don't believe any change in this VS Code integration would be required to take advantage of that once it is complete. However, I don't expect it to be complete anytime soon, and I think it might not be possible to combine "configuration cache" with any command-line flags, so it's less promising than limiting which subprojects get configured.

@nedtwigg
Copy link

I think we (spotless) can make the root :spotlessApply -PspotlessIdeblahblahblah task smart enough to dynamically configure only the required subprojects. diffplug/spotless#623 needs the same thing, so I'll use that as the tracking issue.

@badsyntax
Copy link
Owner Author

badsyntax commented Sep 14, 2020

Thanks for the info! (And sorry for the delayed response.) I'm going to do some digging to spot any optimisations. The "full build" part could potentially be an issue with the vscode gradle tasks extension, which i need to investigate, but I'll also have a look at your suggested approach for spotless optimisations as I am curious.

@badsyntax
Copy link
Owner Author

With a multi-project build, running the root task:

spotlessApply -PspotlessIdeHookUseStdIn -PspotlessIdeHookUseStdOut -PspotlessIdeHook=/Users/richardwillis/Projects/badsyntax/vscode-gradle-fork/gradle-server/src/main/java/com/github/badsyntax/gradle/ByteBufferOutputStream.java --quiet

Shows these logs:

Configure project : started
Configure project : succeeded
Configure project :extension started
Configure project :extension succeeded
Configure project :gradle-server started
Configure project :gradle-server succeeded
Configure project :npm-package started
Configure project :npm-package succeeded
Task :spotlessApply started
Task :spotlessApply UP-TO-DATE
Task :extension:spotlessApply started
Task :extension:spotlessApply UP-TO-DATE
Task :spotlessInternalRegisterDependencies started
Task :spotlessInternalRegisterDependencies UP-TO-DATE
Task :gradle-server:spotlessJava started
Task :gradle-server:spotlessJava skipped
Task :gradle-server:spotlessJavaApply started
Task :gradle-server:spotlessJavaApply skipped
Task :gradle-server:spotlessApply started
Task :gradle-server:spotlessApply SUCCESS
Task :npm-package:spotlessApply started
Task :npm-package:spotlessApply UP-TO-DATE

With a multi-project build, running the sub-project task and the root task:

:gradle-server:spotlessApply :spotlessApply -PspotlessIdeHookUseStdIn -PspotlessIdeHookUseStdOut -PspotlessIdeHook=/Users/richardwillis/Projects/badsyntax/vscode-gradle-fork/gradle-server/src/main/java/com/github/badsyntax/gradle/ByteBufferOutputStream.java --quiet

Shows these logs:

Configure project : started
Configure project : succeeded
Configure project :extension started
Configure project :extension succeeded
Configure project :gradle-server started
Configure project :gradle-server succeeded
Configure project :npm-package started
Configure project :npm-package succeeded
Task :spotlessInternalRegisterDependencies started
Task :spotlessInternalRegisterDependencies UP-TO-DATE
Task :gradle-server:spotlessJava started
Task :gradle-server:spotlessJava skipped
Task :gradle-server:spotlessJavaApply started
Task :gradle-server:spotlessJavaApply skipped
Task :gradle-server:spotlessApply started
Task :gradle-server:spotlessApply SUCCESS
Task :spotlessApply started
Task :spotlessApply UP-TO-DATE

With a multi-project build, running just the sub-project task:

:gradle-server:spotlessApply -PspotlessIdeHookUseStdIn -PspotlessIdeHookUseStdOut -PspotlessIdeHook=/Users/richardwillis/Projects/badsyntax/vscode-gradle-fork/gradle-server/src/main/java/com/github/badsyntax/gradle/ByteBufferOutputStream.java --quiet

Shows these logs:

Configure project : started
Configure project : succeeded
Configure project :extension started
Configure project :extension succeeded
Configure project :gradle-server started
Configure project :gradle-server succeeded
Configure project :npm-package started
Configure project :npm-package succeeded
Task :spotlessInternalRegisterDependencies started
Task :spotlessInternalRegisterDependencies UP-TO-DATE
Task :gradle-server:spotlessJava started
Task :gradle-server:spotlessJava skipped
Task :gradle-server:spotlessJavaApply started
Task :gradle-server:spotlessJavaApply skipped
Task :gradle-server:spotlessApply started
Task :gradle-server:spotlessApply SUCCESS

Running just the sub-project task without the root task seems to be the most efficient. I haven't been able to do a performance check of this change due to all the gradle caching but I assume it's going to give some gains. I can try test on a slow windows VM.

Regarding the "Configure Project" logs, these are emitted no matter which task I execute, and I don't really know what they mean and assume it's just internal to gradle. (These logs are emitted via the Gradle Tooling API.) I'm not sure there's anything I can optimise there.

@nedtwigg i'll watch diffplug/spotless#623 but also happy to make this change in the extension too. It can be an optimisation for users who haven't upgraded their spotless version. What do you think?

@nedtwigg
Copy link

Running just the sub-project task without the root task seems to be the most efficient. I haven't been able to do a performance check of this change due to all the gradle caching but I assume it's going to give some gains. I can try test on a slow windows VM.

This is expected, but it could trigger a bug. The bug is this:

// root build.gradle
target '*.gradle', '*/*.gradle' // format the root gradle scripts, and the gradle scripts of all subprojects

The target above is a bad idea, but it's out there. Spotless allows you to format any file "below" you, so if a file is in the :subproject directory, it's still possible that it's being formatted by a parent. You can safely exclude siblings though.

Regarding the "Configure Project" logs, these are emitted no matter which task I execute

If you set org.gradle.configureondemand=true in your gradle.properties, then you will get a further speed boost, which you can see reflected in fewer "Configure project :blah" logs. It is not yet enabled by default because some projects which rely on lots of cross-project configuration can be broken by it.

I'll watch diffplug/spotless#623 but also happy to make this change in the extension too. It can be an optimisation for users who haven't upgraded their spotless version.

This is a great idea. First off, you are 100% correct that it will be a speedup for all versions of Spotless that support the IDE hook. I'd say you don't even need to follow 623 anymore, because I don't think it can go any faster than you can. It would be the same speed, just less work for you to integrate. But if you've already done the integration work, why add more to detect "such and such version", since there's no benefit. I also don't have any plans to implement 623 anytime soon.

@badsyntax
Copy link
Owner Author

Thanks for the clarifications!

Cool, I'll have a bash an implementing the change in this extension 👍

Regarding the logic to detect which (sub)project a file belongs to, it looks like this might be a bit tricky, as you can rename the build file to something other than the standard convention (build.gradle). Here's an example.

It seems the best way to do this is to use the Gradle API to figure this out. Example here. AFAIK, the Gradle Tooling API doesn't expose similar API methods, and I'd need to add a custom plugin/model to the build (via an init script) in order to query the build. I have a (very rough) POC for adding a custom plugin & model to the build, but I remember having issues by not being able to debug the plugin. Either way, I will need to do further investigations before deciding on the correct approach here. Sadly it doesn't seem straightforward.

@nedtwigg
Copy link

If the instructions to users for getting the speedup are "apply spotless, and then also apply this other plugin in your init.gradle", then I would think that it would be better to just get :spotlessApply -PspotlessIdeHook=blah to be smarter. But I know you have other VSCode gradle plugins, so maybe the init script fixes problems you have in your other plugins too.

@badsyntax
Copy link
Owner Author

I really don't want to go the init script route, but as you point out it does open up the possibility for other features in the main vscode-gradle extension.

At a very high-level, and without thinking about it too deeply, here's the concept:

  • Add the custom plugin to the build via init script
  • Add a new grpc method/endpoint to return the sub-project for a specified file
    • This method will call the custom plugin added to the build to retrieve the project name
  • Add a new public API method getProject(file) to the public vscode API of the main vscode-gradle extension
  • Call getProject(file) from this extension before running the spotless apply task.
  • Adjust the spotlessApply arguments accordingly depending on the result of getProject(file)

But now that I type that out, i see that running that additional logic in gradle could result in a performance hit. So it's hard to say it's worth the effort.

I am tempted to scratch that plan, and just do a simple directory traversal looking for common build files (build.gradle & build.gradle.kts), and ignore projects with custom build file names. I'm tempted to say the majority of projects don't have custom build file names, and if they do, it's not the end of the world, they just don't get this performance gain.

@nedtwigg
Copy link

and if they do, it's not the end of the world, they just don't get this performance gain

You'll need to also detect if they are using some non-standard layout, and bail out of only calling the subprojects, and instead call the full-blown spotlessApply, rather than :spotlessApply :subproject:spotlessApply. It's not just a performance optimization - you will get an incorrect result if you don't correctly detect the .gradle files.

I semi-frequently create multi-project setups where the only build.gradle is in the root project, and just subproject blocks for the subprojects.

IMO, this is shaping up to be complicated enough that :spotlessApply ought to get smarter. It's the best possible performance, and it also looks to be the easiest place to implement it.

@badsyntax
Copy link
Owner Author

I semi-frequently create multi-project setups where the only build.gradle is in the root project, and just subproject blocks for the subprojects.

I didn't know you could do this :(

IMO, this is shaping up to be complicated enough that :spotlessApply ought to get smarter. It's the best possible performance, and it also looks to be the easiest place to implement it.

Yea, with this new information it does seem like doing this with spotless would be easiest and best. I guess i'll just keep an eye out on diffplug/spotless#623. If i have time to get familiar with the spotless codebase i might try contribute. I might also try solve this "client side" as i am curious, but it's a chunk of work so i can't say when i can get around to trying it.

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

No branches or pull requests

2 participants