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.

Sign up for GitHub

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

Jump to bottom

GradleProject model should not include the task lists. #8898

Open
ducrohet opened this issue Mar 29, 2019 · 34 comments
Open

GradleProject model should not include the task lists. #8898

ducrohet opened this issue Mar 29, 2019 · 34 comments
Labels
a:feature A new functionality in:tooling-api

Comments

@ducrohet
Copy link
Contributor

org.gradle.tooling.model.GradleProject is the key entry point for syncing project in IntelliJ (IJ) and Android Studio (AS). We need to query this model first before querying our more specialized models.

Currently, it also contains the list of tasks. This is used in IJ/AS to display the task in the Gradle pane on the right.

We would like to make this optional somehow. Maybe make the current model always return an empty list, and provide a different model that returns the task list if needed.
Our reasoning:

  • Nobody uses this in Studio (don't know about IJ). We don't have actual metrics (we will add some) but users have told us they don't use it.
  • This does not scale. A 100 module project will create 20,000 tasks. We have heard from users with several 100s of modules that this is something that impact performance and are doing hacks to disable it on the IDE side.
  • This triggers task configuration during sync because GradleTask includes the group and description that requires a task configuration. Side note, it would be nice to be able to set these, as well as dependencies on TaskProvider, but in our case it's just simpler to not return the tasks at all.

A different option might be to only provide the tasks that have a group associated with them. Right now it includes all the tasks, including the ones that don't have groups. None of these are meant to be called directly (in the case of Android at least). Doing this would require us to be able to set group on TaskProvider to avoid triggering configuration.
@tasomaniac
Copy link

Also an overloaded register function to provide the task group earlier would be beneficial. In current API, tasks are needed to be realized and configured in order to determine their groups. This leads all tasks to be configured which is not necessary even when users are indeed using the pane.

@donat
Copy link
Member

donat commented Apr 24, 2019 •
edited

@ducrohet There's already a Tooling API model (GradleBuild) that describes the project layout without the available tasks. Is this model useful for you or does it have missing features?

@donat donat closed this as completed Apr 30, 2019
@big-guy big-guy reopened this Apr 30, 2019
@big-guy
Copy link
Member

big-guy commented Apr 30, 2019

Let's leave this open until we hear back from Xav.

@ducrohet
Copy link
Contributor Author

ducrohet commented Apr 30, 2019 •
edited

It turns out that IntelliJ does not actually use GradleProject so we will attempt to fix this there.

However I think the bug still stands. BasicGradleProject is a good replacement for GradleProject though missing getBuildDirectory to be really useful.

Also the comment about group/description on TaskProvider still stands as well :)

@ducrohet ducrohet mentioned this issue Jun 28, 2019
Open
@ducrohet
Copy link
Contributor Author

ducrohet commented Aug 9, 2019

We're currently looking into this more actively. IntelliJ has its own way to populate its task list window and we can (and will) disable that (or put it behind a setting at least.)

However, it does query for IdeaModel in the tooling action which internally gets created by building the hierarchy of GradleProject which builds the list of tasks.

We are going to need to find a way to do this. One possibility would be to include a parameter to call getModel() with on BuildController to basically disable task creation. Would that be acceptable?

@donat
Copy link
Member

donat commented Sep 4, 2019

Sorry for the late reply.

So, if I understand correctly, you need to change the content of the GradleProject model, because IDEA uses the same model internally. I guess using a different model would require more changes in Android Studio.

Parameterizing the corresponding model builder would be an option, but it requires a bit of design. I'm planning to create a Gradle test snapshot for which you can set a system property that disables the task creation. With that, you can measure the impact of skipping the task creation. How does that sound?

@ducrohet
Copy link
Contributor Author

ducrohet commented Sep 5, 2019 •
edited by donat

Hi,

we can do some test with a flag, that would be a good way to look at the impact of this. I think with regard to an actual solution, moving to something that's compatible with TaskProvider is probably a better solution. I filed Issue #9823 for this a while back.

@donat
Copy link
Member

donat commented Sep 11, 2019 •
edited

I've created patched Gradle 5.6 release in which you can influence the GradleProjectBuilder class with the GradleProjectOptions system property. The possible values are:

  • unmodified: the original behavior
  • skip_task_serialization load the tasks in the model builder but don't send it to the client
  • skip_task_load: don't realize the tasks

I've used the Gradle profiler in conjunction with the perf-android-large project to measure the performance difference. I used the following scenario file for the measurement:

gradle_project {
  model = "org.gradle.tooling.model.GradleProject"
}

gradle_project_without_serializing_tasks {
  model = "org.gradle.tooling.model.GradleProject"
  system-properties {
    GradleProjectOptions = "skip_task_serialization"
  }
}

gradle_project_without_loading_tasks {
  model = "org.gradle.tooling.model.GradleProject"
  system-properties {
    GradleProjectOptions = "skip_task_load"
  }
}

Running the scenarios with 5 warm-ups and 50 iterations each I got the following results:

Value gradle_project without_serializing_tasks without_loading_tasks
mean(ms) 1328.36 1305.16 903.92
stddev(ms) 146.080319 152.78 157.59

It seems that the serialisation is negligible in the tested scenario, and realising the task graph itself is also acceptable, just a few hundred milliseconds for a project with ~21.000 tasks.

Of course, this is not conclusive as the tested project is a synthetic one. Do you have a large, real-life project at hand on which you can reproduce this measurement?

@tasomaniac
Copy link

Also keep in mind that this operation in IDEs are not run that often. Only when Gradle files actually change.

@stephanenicolas
Copy link

@tasomaniac opening the IDE / project and switching branches also triggers this, and it makes Android Studio unususable for a few minutes in projects with 1k modules.

@ducrohet
Copy link
Contributor Author

ducrohet commented Oct 3, 2019

@donat It seems the link to the patched 5.6 is gone?

@donat
Copy link
Member

donat commented Oct 4, 2019

Interesting. I'll recreate it for you today.
Or, if you need it faster, you can rebuild my version from the donat/remove-tasks-from-tapi-model branch with the ./gradlew install -Pgradle_installPath=/path/to/dist command.

@stephanenicolas
Copy link

@ducrohet if this makes it in gradle, will we have a UI button to enable/disable the task list from Android Studio ?

@donat
Copy link
Member

donat commented Oct 8, 2019

Better late than never, here's the patched distribution you can test with: https://services.gradle.org/distributions-snapshots/gradle-6.1-branch-donat_remove_tasks_from_tapi_model-20191004160515+0000-bin.zip

@stephanenicolas
Copy link

stephanenicolas commented Oct 8, 2019 •
edited

@donat , I tried your version. It works fine to build our project on the command line but I get this issue in Android Studio: '6.1-branch-donat_remove_tasks_from_tapi_model-20191004160515+0000' is not a valid Gradle version string (examples: '1.0', '1.0-rc-1')

I tried renaming the gradle distribution to change the version to 6.1, but it didn't work. Here is the full idea.log stack trace. Is the gradle version embedded in the jar ?

org.gradle.tooling.GradleConnectionException: Could not create an instance of Tooling API implementation using the specified Gradle distribution 'file:///<path>/gradle-6.1-bin.zip'.
        at org.gradle.tooling.internal.consumer.loader.DefaultToolingImplementationLoader.create(DefaultToolingImplementationLoader.java:114)
        at org.gradle.tooling.internal.consumer.loader.CachingToolingImplementationLoader.create(CachingToolingImplementationLoader.java:44)
        at org.gradle.tooling.internal.consumer.loader.SynchronizedToolingImplementationLoader.create(SynchronizedToolingImplementationLoader.java:43)
        at org.gradle.tooling.internal.consumer.connection.LazyConsumerActionExecutor.onStartAction(LazyConsumerActionExecutor.java:101)
        at org.gradle.tooling.internal.consumer.connection.LazyConsumerActionExecutor.run(LazyConsumerActionExecutor.java:83)
        at org.gradle.tooling.internal.consumer.connection.CancellableConsumerActionExecutor.run(CancellableConsumerActionExecutor.java:45)
        at org.gradle.tooling.internal.consumer.connection.ProgressLoggingConsumerActionExecutor.run(ProgressLoggingConsumerActionExecutor.java:58)
        at org.gradle.tooling.internal.consumer.connection.RethrowingErrorsConsumerActionExecutor.run(RethrowingErrorsConsumerActionExecutor.java:38)
        at org.gradle.tooling.internal.consumer.async.DefaultAsyncConsumerActionExecutor$1$1.run(DefaultAsyncConsumerActionExecutor.java:55)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:63)
        at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:46)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:55)
        at java.lang.Thread.run(Thread.java:748)
        at org.gradle.tooling.internal.consumer.BlockingResultHandler.getResult(BlockingResultHandler.java:46)
        at org.gradle.tooling.internal.consumer.DefaultModelBuilder.get(DefaultModelBuilder.java:50)
        at org.jetbrains.plugins.gradle.service.execution.GradleExecutionHelper.getBuildEnvironment(GradleExecutionHelper.java:577)
        at org.jetbrains.plugins.gradle.service.execution.GradleExecutionHelper.getBuildEnvironment(GradleExecutionHelper.java:86)
        at org.jetbrains.plugins.gradle.service.project.GradleProjectResolver.doResolveProjectInfo(GradleProjectResolver.java:181)
        at org.jetbrains.plugins.gradle.service.project.GradleProjectResolver.access$200(GradleProjectResolver.java:73)
        at org.jetbrains.plugins.gradle.service.project.GradleProjectResolver$ProjectConnectionDataNodeFunction.fun(GradleProjectResolver.java:736)
        at org.jetbrains.plugins.gradle.service.project.GradleProjectResolver$ProjectConnectionDataNodeFunction.fun(GradleProjectResolver.java:719)
        at org.jetbrains.plugins.gradle.service.execution.GradleExecutionHelper.execute(GradleExecutionHelper.java:227)
        at org.jetbrains.plugins.gradle.service.project.GradleProjectResolver.resolveProjectInfo(GradleProjectResolver.java:138)
        at org.jetbrains.plugins.gradle.service.project.GradleProjectResolver.resolveProjectInfo(GradleProjectResolver.java:73)
        at com.intellij.openapi.externalSystem.service.remote.RemoteExternalSystemProjectResolverImpl.lambda$resolveProjectInfo$0(RemoteExternalSystemProjectResolverImpl.java:36)
        at com.intellij.openapi.externalSystem.service.remote.AbstractRemoteExternalSystemService.execute(AbstractRemoteExternalSystemService.java:57)
        at com.intellij.openapi.externalSystem.service.remote.RemoteExternalSystemProjectResolverImpl.resolveProjectInfo(RemoteExternalSystemProjectResolverImpl.java:36)
        at com.intellij.openapi.externalSystem.service.remote.wrapper.ExternalSystemProjectResolverWrapper.resolveProjectInfo(ExternalSystemProjectResolverWrapper.java:44)
        at com.intellij.openapi.externalSystem.service.internal.ExternalSystemResolveProjectTask.doExecute(ExternalSystemResolveProjectTask.java:99)
        at com.intellij.openapi.externalSystem.service.internal.AbstractExternalSystemTask.execute(AbstractExternalSystemTask.java:165)
        at com.intellij.openapi.externalSystem.service.internal.AbstractExternalSystemTask.execute(AbstractExternalSystemTask.java:151)
        at com.intellij.openapi.externalSystem.util.ExternalSystemUtil$3.executeImpl(ExternalSystemUtil.java:563)
        at com.intellij.openapi.externalSystem.util.ExternalSystemUtil$3.lambda$execute$0(ExternalSystemUtil.java:399)
        at com.intellij.openapi.project.DumbServiceImpl.suspendIndexingAndRun(DumbServiceImpl.java:146)
        at com.intellij.openapi.externalSystem.util.ExternalSystemUtil$3.execute(ExternalSystemUtil.java:399)
        at com.intellij.openapi.externalSystem.util.ExternalSystemUtil$5.run(ExternalSystemUtil.java:668)
        at com.intellij.openapi.progress.impl.CoreProgressManager$TaskRunnable.run(CoreProgressManager.java:731)
        at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$runProcess$2(CoreProgressManager.java:164)
        at com.intellij.openapi.progress.impl.CoreProgressManager.registerIndicatorAndRun(CoreProgressManager.java:586)
        at com.intellij.openapi.progress.impl.CoreProgressManager.executeProcessUnderProgress(CoreProgressManager.java:532)
        at com.intellij.openapi.progress.impl.ProgressManagerImpl.executeProcessUnderProgress(ProgressManagerImpl.java:86)
        at com.intellij.openapi.progress.impl.CoreProgressManager.runProcess(CoreProgressManager.java:151)
        at com.intellij.openapi.progress.impl.CoreProgressManager$4.run(CoreProgressManager.java:403)
        at com.intellij.openapi.application.impl.ApplicationImpl$1.run(ApplicationImpl.java:312)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.IllegalArgumentException: '6.1-branch-donat_remove_tasks_from_tapi_model-20191004160515+0000' is not a valid Gradle version string (examples: '1.0', '1.0-rc-1')
        at org.gradle.util.GradleVersion.<init>(GradleVersion.java:110)
        at org.gradle.util.GradleVersion.version(GradleVersion.java:101)
        at org.gradle.tooling.internal.consumer.versioning.VersionDetails.from(VersionDetails.java:35)
        at org.gradle.tooling.internal.consumer.connection.CancellableConsumerConnection.<init>(CancellableConsumerConnection.java:36)
        at org.gradle.tooling.internal.consumer.connection.ShutdownAwareConsumerConnection.<init>(ShutdownAwareConsumerConnection.java:30)
        at org.gradle.tooling.internal.consumer.connection.TestExecutionConsumerConnection.<init>(TestExecutionConsumerConnection.java:33)
        at org.gradle.tooling.internal.consumer.connection.ParameterAcceptingConsumerConnection.<init>(ParameterAcceptingConsumerConnection.java:34)
        at org.gradle.tooling.internal.consumer.connection.PhasedActionAwareConsumerConnection.<init>(PhasedActionAwareConsumerConnection.java:46)
        at org.gradle.tooling.internal.consumer.loader.DefaultToolingImplementationLoader.create(DefaultToolingImplementationLoader.java:93)
        at org.gradle.tooling.internal.consumer.loader.CachingToolingImplementationLoader.create(CachingToolingImplementationLoader.java:44)
        at org.gradle.tooling.internal.consumer.loader.SynchronizedToolingImplementationLoader.create(SynchronizedToolingImplementationLoader.java:43)
        at org.gradle.tooling.internal.consumer.connection.LazyConsumerActionExecutor.onStartAction(LazyConsumerActionExecutor.java:101)
        at org.gradle.tooling.internal.consumer.connection.LazyConsumerActionExecutor.run(LazyConsumerActionExecutor.java:83)
        at org.gradle.tooling.internal.consumer.connection.CancellableConsumerActionExecutor.run(CancellableConsumerActionExecutor.java:45)
        at org.gradle.tooling.internal.consumer.connection.ProgressLoggingConsumerActionExecutor.run(ProgressLoggingConsumerActionExecutor.java:58)
        at org.gradle.tooling.internal.consumer.connection.RethrowingErrorsConsumerActionExecutor.run(RethrowingErrorsConsumerActionExecutor.java:38)
        at org.gradle.tooling.internal.consumer.async.DefaultAsyncConsumerActionExecutor$1$1.run(DefaultAsyncConsumerActionExecutor.java:55)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:63)

Can I simply add this to our gradle.properties file to test the fork:

skip_task_serialization=true
skip_task_load=true

Or I should hack the init.gradle file ?

@donat
Copy link
Member

donat commented Oct 8, 2019

@stephanenicolas I think the easiest is to rebuild my fork locally. It might take a few minutes, but the result version should be accepted by the Tooling API.

@stephanenicolas
Copy link

@donat I will try later today. I never built gradle yet..
Was I right in how to pass the 2 params's values you introduced ?

@donat
Copy link
Member

donat commented Oct 8, 2019 •
edited

Almost. The value should be specified as a system property (and not as a project property). The correct gradle.properties file should look like this:

systemProp.GradleProjectOptions=skip_task_load

or this:

systemProp.GradleProjectOptions=skip_task_serialization

@stephanenicolas
Copy link

stephanenicolas commented Oct 8, 2019 via email

@stephanenicolas
Copy link

stephanenicolas commented Oct 8, 2019 •
edited

@donat I manged to build your branch, thx for the help. And AS is using it.
Though: The sync time is unchanged and the task list is still filled in AS.

I am not sure the system properties are correctly passed during a sync.
Can it be related to #6825 ?

It is really strange because I even hacked your branch to hard code all the conditions: it never computes the list of tasks and even less realize them. Though, the list of tasks is still there in AS, and the sync times are almost unchanged. I am 100% sure that I am using the gradle fork from AS.
I must be missing something here...

@stephanenicolas
Copy link

I think I understood the problem: the sync doesn't follow the gradle version used to build the app:

initscript {
  dependencies {
    classpath files(["/Users/<>/Library/Application Support/AndroidStudio3.5/Kotlin/lib/kotlin-gradle-tooling.jar","/Users/<>/Library/Application Support/AndroidStudio3.5/Kotlin/lib/sam-with-receiver-ide-plugin.jar","/Users/<>/Library/Application Support/AndroidStudio3.5/Kotlin/lib/kotlin-plugin.jar","/Users/<>/Library/Application Support/AndroidStudio3.5/Kotlin/lib/noarg-ide-plugin.jar","/Applications/Android Studio.app/Contents/lib/kotlin-stdlib-1.3.50.jar","/Users/<>/Library/Application Support/AndroidStudio3.5/Kotlin/lib/kapt3-idea.jar","/Applications/Android Studio.app/Contents/plugins/android/lib/android-extensions-ide.jar","/Applications/Android Studio.app/Contents/lib/external-system-rt.jar","/Applications/Android Studio.app/Contents/plugins/gradle/lib/gradle-tooling-extension-api.jar","/Applications/Android Studio.app/Contents/plugins/gradle/lib/gradle-api-impldep-4.10.3.jar","/Users/<>/Library/Application Support/AndroidStudio3.5/Kotlin/lib/allopen-ide-plugin.jar","/Applications/Android Studio.app/Contents/lib/gson-2.8.5.jar","/Applications/Android Studio.app/Contents/lib/groovy-all-2.4.15.jar","/Applications/Android Studio.app/Contents/plugins/gradle/lib/gradle-tooling-extension-impl.jar"])
  }
}

It uses the tooling apis shipped with AS... How can we change that ? @donat

@donat
Copy link
Member

donat commented Oct 10, 2019

AFAIC it can only be done by rebuilding AS from the sources. This is only an experiment thought. Why don't you change my branch and hard-code the removal of the task graph removal from the model, rebuild the Gradle distribution and use it in your AS project? You only need to comment out the parts that are guarded by the GradleProjectOptions system property.

@stephanenicolas
Copy link

stephanenicolas commented Oct 10, 2019 via email

@donat donat pinned this issue Nov 20, 2019
@donat donat unpinned this issue Nov 20, 2019
@donat
Copy link
Member

donat commented Nov 20, 2019

I'm actually looking into how can we profile Gradle in the IDE-context.

@donat
Copy link
Member

donat commented Nov 20, 2019

@ducrohet

We're currently looking into this more actively. IntelliJ has its own way to populate its task list window and we can (and will) disable that (or put it behind a setting at least.)

AFAIU Android Studio is a fork of IDEA, right? If that's true, you can just replace loading GradleProject model with a custom one that does the same thing except loading the tasks. That solution would be also compatible with existing Gradle releases.

@ducrohet
Copy link
Contributor Author

It is not a fork. It's a custom distribution of IDEA with the Android plugin and some custom productization (name, splashcreen and some custom UI around projects management to be Android only).

The code of the platform is untouched.

@donat
Copy link
Member

donat commented Nov 21, 2019

Good to know. Now, I understand the situation better.

@donat
Copy link
Member

donat commented Dec 17, 2019

For the sake of completeness, the PR for introducing system properties to disable task creation is merged.

@stephanenicolas
Copy link

stephanenicolas commented Dec 18, 2019 via email

@donat
Copy link
Member

donat commented Jan 20, 2020

@ducrohet @stephanenicolas Can you please share some numbers on the performance gained in your test projects?

@stephanenicolas
Copy link

stephanenicolas commented Jan 20, 2020 via email

@stale
Copy link

stale bot commented Jan 19, 2021

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. If you're interested in how we try to keep the backlog in a healthy state, please read our blog post on how we refine our backlog. If you feel this is something you could contribute, please have a look at our Contributor Guide. Thank you for your contribution.

@stale stale bot added the stale label Jan 19, 2021
@eskatos eskatos removed the stale label Jan 19, 2021
@stale
Copy link

stale bot commented Mar 23, 2022

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. If you're interested in how we try to keep the backlog in a healthy state, please read our blog post on how we refine our backlog. If you feel this is something you could contribute, please have a look at our Contributor Guide. Thank you for your contribution.

@stale stale bot added the stale label Mar 23, 2022
@tasomaniac
Copy link

As far as I know Android Studio did improvements on this area. The task list is no longer generated on sync which speeds up sync time. But that comes with a disadvantage since the tasks are no longer visible in the IDE and does not show up in autocompletion either.

That's why I don't think this issue is stale. It would be nice to benefit from this speed improvement while having just a list of task names available

@stale stale bot removed the stale label Mar 23, 2022
@ov7a ov7a added the a:feature A new functionality label Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a:feature A new functionality in:tooling-api
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@eskatos @donat @tasomaniac @stephanenicolas @jjohannes @ducrohet @ov7a @big-guy