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

Split out help tasks into a separate plugin or remove (some of) them #130

Open
TheGoesen opened this issue Sep 29, 2024 · 7 comments
Open
Milestone

Comments

@TheGoesen
Copy link
Contributor

TheGoesen commented Sep 29, 2024

This plugin uses registerTask instead of createTask. Thats sufficient for "normal" gradle build to avoid unnecessary overhead, as the tasks are simply not created if not needed.
However during an IDE/IntelliJ sync all tasks are actually created. This is can lead to reduced performance, especially when you have a look at all that stuff that happens in the constructor of ModuleVersionRecommendation which normally would be avoided if you dont call that target directly, however during a sync especially the creation of a new configuration at this stage can lead to odd sideeffects

@jjohannes
Copy link
Member

I don't like the idea to removing tasks through options. It's not very idiomatic from my understanding. Plugins should register as many tasks as they like/need (and that should be cheap) and then Gradle should figure out which tasks are actually needed in a certain situation. There should not be additional options a user has to think about.

What we could consider in the future is splitting the plugin up into two:

  • One plugin with the minimal base functionality
  • One plugin providing the "helper" tasks

In any case, if the constructor of ModuleVersionRecommendation is too expensive, we should check how to move code from there into the task execution phase. Can you please share, which code is causing the issue?

@jjohannes jjohannes added the pending:feedback Issues waiting for feedback from the reporter label Oct 10, 2024
@jjohannes jjohannes changed the title Option to disable some non build related tasks? [Maybe] Optimize task creation performance of ModuleVersionRecommendation Oct 10, 2024
@TheGoesen
Copy link
Contributor Author

TheGoesen commented Oct 10, 2024

and then Gradle should figure out which tasks are actually needed in a certain situation.

I would agree with that. But this is not how the IntelliJ Project sync works currently. The Intellij Project sync creates every single task of every single project. Maybe we should crossreport this to Jetbrains?
The line of code that gave me project some hickups was this
Configuration latestVersionsClasspath = configurations.create("latestVersionsClasspath", c -> {
since the configuration "latestVersionClasspath" is created and not just registered, everything that has been registered using configurations.configureEach will trigger immediatly at a very strange time to be triggered. Usually thats not an issue because if you are "just using gradle" the task will never be created so there is also no latestVersionClasspath. Then everything will work fine until you hit the intellij resync button and bam!

@tg-freigmbh
Copy link

@jjohannes jjohannes changed the title [Maybe] Optimize task creation performance of ModuleVersionRecommendation Split out help tasks into a separate plugin or remove (some of) them Oct 11, 2024
@jjohannes
Copy link
Member

Thank you for the additional thoughts.

The more I think about it, I think we should split the tasks out and put them into a separate plugin that users can use if they need them. TBH, I don't know if anyone uses these tasks. I added them in early stages of the plugin more as an experiment to see what can be done and if it is useful. I still think some of the tasks could be useful if you want to migrate to using this plugin (or migrate away from using it). But that's a one time usage of such tasks and I don't have any data on how useful they really are in practice.

Regarding IntelliJ:

  • Android Studio already has an option to not create the task tree on sync. Which is also not super nice, but helpful for large projects. I don't know if there are plans to backport this to vanilla IntelliJ.
  • I think IntelliJ can't solve this without adjustments in Gradle itself. The problem is that they need to read the group and description properties of the tasks. And these are configured as part of the task configuration. Currently, there is no way to get these values without creating all the task objects as the configuration may happen anywhere (include the tasks' constructors). To improve this, Gradle would need to change this API, such that group and description and not properties of every task, but something "special" that is configured outside of the task (e.g. as specific parameters to the register method). Similar to how the "name" is configured outside of the task.

@jjohannes jjohannes removed the pending:feedback Issues waiting for feedback from the reporter label Oct 11, 2024
@jjohannes jjohannes added this to the 2.0 milestone Nov 4, 2024
@jjohannes
Copy link
Member

Starting with 1.8, you can add the following to gradle.properties to deactivate the tasks:

org.gradlex.java-module-dependencies.register-help-tasks=false

I did it like this, because removing them from the plugin is a quite a breaking change. I'll leave this issue open for further work on 2.0. Then, we should remove the property again and decide which...

  • tasks to keep in this plugin if any (maybe moduleDependencies, analyzeModulePath).
  • tasks to remove completely (probably checkAllModuleInfo, check*ModuleInfo, recommendModuleVersions)
  • tasks to move to a separate plugin (something like java-module-migration). This can maybe even go into a separate git repository as such tasks that rewrite files in the project may be used independent of this plugin. Or just remove these as well. They are quite experimental at this stage.

@tg-freigmbh
Copy link

Change seems good! Side note: some tasks are also shiped by extra-module-info..
grafik

@jjohannes
Copy link
Member

Change seems good! Side note: some tasks are also shipped by extra-module-info..

Yes. It's the same "issue" there. I think it only adds that one task. But it should ideally also be moved to a separate plugin.

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

No branches or pull requests

3 participants