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

Support Gradle configuration cache #228

Closed
LouisCAD opened this issue Aug 24, 2020 · 10 comments · Fixed by #495
Closed

Support Gradle configuration cache #228

LouisCAD opened this issue Aug 24, 2020 · 10 comments · Fixed by #495
Assignees
Labels
help wanted Extra attention is needed
Milestone

Comments

@LouisCAD
Copy link
Member

LouisCAD commented Aug 24, 2020

Relevant documentation: https://docs.gradle.org/nightly/userguide/configuration_cache.html

@julioz
Copy link

julioz commented Jan 28, 2021

Hi, we're exploring configuration caching at SC; is this something you have planned on working soon, have an estimate you could provide, or a description of what prevents this work from happening?

@LouisCAD
Copy link
Member Author

Hello, sorry for the late response.

I can't give any promises, but I hope we can address this issue in 2021.

@pikzen
Copy link

pikzen commented Oct 12, 2021

As a little update on this: the gradle configuration cache currently is very unhappy about this plugin using buildFinished (https://github.com/jmfayard/refreshVersions/blob/main/plugins/core/src/main/kotlin/de/fayard/refreshVersions/core/internal/RefreshVersionsConfigHolder.kt#L132). I'll be trying to look into how this can be fixed.

@LouisCAD
Copy link
Member Author

How is it unhappy?

@pikzen
Copy link

pikzen commented Oct 12, 2021

When running our project build (that makes use of refreshVersions) with --configuration-cache, this is the output:

2: Task failed with an exception.

  • What went wrong:
    Configuration cache problems found in this build.

1 problem was found storing the configuration cache.

See the complete report at file:https:///.../configuration-cache/as0e4uasippp0jkgc90s5xhk-1/configuration-cache-report.html

Listener registration 'Gradle.buildFinished' by build is unsupported.

Note that Gradle is as helpful as ever, and I've had to actually jump into debugging mode with it to learn that it came from this plugin in our project.

When running with configuration cache problems only shown as warnings, it technically caches it, but from experience with the build cache, it'll lead to some tasks not being re-run/run with the same configuration as previously. I am currently investigating how to remove those buildFinished calls, by registering a service with gradle that'll handle it, but since those buildFinished calls come from a place where there is no available Project, but just a ProjectDescriptor, this might get a little bit hairy.

EDIT: nvm, it can be registered through settings too. I'll keep investigating and come back with findings.

EDIT 2: That was surprisingly easy. Taking inspiration from what gradle-doctor did (runningcode/gradle-doctor@120d920), but by passing lambdas instead of requiring everything to be an autocloseable, allowing us to do any needed cleanup

    abstract class StaticStateClearOnBuildFinishedService : BuildService<BuildServiceParameters.None>, AutoCloseable {
        private val closeList = LinkedList<() -> Unit>()

        fun closeMeWhenFinished(lambda: () -> Unit) {
            closeList.push(lambda)
        }

        override fun close() {
            closeList.forEach {
                it()
            }

            closeList.clear()
        }

    }

then, in the places where clearStaticState was called, simply attempt to use that service when gradle allows it.

     if (GradleVersion.version(settings.gradle.gradleVersion) >= GradleVersion.version("6.5")) {
            // Closeable services were integrated in 6.5
            val closeService = settings.gradle.sharedServices
                .registerIfAbsent("close-resources-service", StaticStateClearOnBuildFinishedService::class.java) {}.get()

            closeService.closeMeWhenFinished { httpClient.dispatcher.executorService.shutdown() }
            closeService.closeMeWhenFinished { resettableDelegates.reset() }
        } else {
            settings.gradle.buildFinished {
                clearStaticState()
            }
        }

For tests, I had added a little bit of logging to ensure our lambda gets called, and it does. (Ignore the build failure, no phone plugged in):
./gradlew --configuration-cache application:installDevelopmentDebug

[about 200 modules worth of build info]
> Task :application:installDevelopmentDebug FAILED
Closing!
Closing!

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':application:installDevelopmentDebug'.
> com.android.builder.testing.api.DeviceException: No connected devices!

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 11m 37s
2803 actionable tasks: 1813 executed, 984 from cache, 6 up-to-date
Configuration cache entry stored.

Future runs do seem to reuse that cache:

./gradlew --configuration-cache installDevelopmentDebug
Configuration cache is an incubating feature.
Reusing configuration cache.
> Task :application:installDevelopmentDebug FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':application:installDevelopmentDebug'.
> com.android.builder.testing.api.DeviceException: No connected devices!

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 12s
2803 actionable tasks: 1 executed, 2802 up-to-date
Configuration cache entry reused.

Let me know if you'd like me to make this a pull request, or if you'd rather handle it/test yourself.

@LouisCAD
Copy link
Member Author

Thanks for your research!
That'll be helpful when I'll spend time on this issue.

My current focus is supporting automatic replacement of deprecated dependency notations, and it's a days long effort I'm trying to fit into my free time when I have the energy.

Once I'm done with it, this issue will likely be top priority, besides updating some dependency notations in AndroidX again for new WearOS artifacts.

@yogurtearl
Copy link

Also, you should make sure to test config cache with every task that the plugin has.

E.g. :refreshVersionsMigrate has these config cache issues:

Invocation of 'Task.project' by task ':refreshVersionsMigrate' at execution time is unsupported.
Have to grap the needed values during configuration and pass them in as @Input

here:
https://github.com/jmfayard/refreshVersions/blob/873210a27849ad252af8d561d042d8730d23190e/plugins/dependencies/src/main/kotlin/de/fayard/refreshVersions/RefreshVersionsMigrateTask.kt#L16

and here:
https://github.com/jmfayard/refreshVersions/blob/873210a27849ad252af8d561d042d8730d23190e/plugins/dependencies/src/main/kotlin/de/fayard/refreshVersions/RefreshVersionsMigrateTask.kt#L24

Ideally this would be calculated at configuration time and passed an @Input to the task:
https://github.com/jmfayard/refreshVersions/blob/873210a27849ad252af8d561d042d8730d23190e/plugins/core/src/main/kotlin/de/fayard/refreshVersions/core/MissingVersionEntries.kt#L51-L65

@LouisCAD LouisCAD added this to the v1.0.0 milestone Dec 19, 2021
@LouisCAD
Copy link
Member Author

Hello, FYI, I'm working on this issue right now, and I expect it to take a few days to complete.

If one of you want to join in a pair-programming session, let me know, I'll be happy to have one of you onboard!

@LouisCAD LouisCAD self-assigned this Jan 23, 2022
@LouisCAD
Copy link
Member Author

Thanks @pikzen for demonstrating it to work.
I checked the doc, tried something similar, did some testing, and it works well so long you don't try to run the refreshVersions task with configuration cache enabled. That's already a big improvement as it no longer prevents you from using configuration cache in the build.

I'll see if I can quickly handle the issue affecting the refreshVersions task before the impending release.

@LouisCAD
Copy link
Member Author

It's impossible (and pointless) to support Gradle configuration cache for the refreshVersions task (see why in the description here), so I'm dropping this part.

The main goal is to allow projects to take advantage of configuration cache safely, and this has been implemented successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants