Skip to content

Improve Gradle build cacheability#15483

Merged
jdaugherty merged 3 commits intoapache:7.0.xfrom
jprinet:7.0.x
Mar 11, 2026
Merged

Improve Gradle build cacheability#15483
jdaugherty merged 3 commits intoapache:7.0.xfrom
jprinet:7.0.x

Conversation

@jprinet
Copy link
Contributor

@jprinet jprinet commented Mar 3, 2026

Description

This PR improves Gradle build cacheability by

  • Removing the non-required base.dir absolute path from Test.systemProperties
  • Ignoring sbom.json during cache key computation
  • Migrating GroovyDoc.doFirst block to a dedicated task [Optional as this breaks configuration cache compatibility]

Details

Running experiment 3 of the automated validation scripts helped to discover 587 cacheable tasks executed in 1h 31m 58.817s of serial time in the second Build Scan.
In ideal situations, no cacheable task should run in the second build of an experiment, as they should all come from the cache.

The cache misses reasons illustrated in this build scan comparison are diverse

Here are the patterns

  • GroovyDoc: using doFirst breaks cacheability (doc), however, the remediation implemented breaks configuration cache compatibility . I believe it's still worth it given CC can't be enabled for other reasons
  • GroovyCompile, Checkstyle: classpath differences due to the sbom.json file
  • GroovyCompile: usage of doFirst
  • Test: systemProperty differences due to base.dir absolute path

Applying the fix reduced the number of cache misses to 242 tasks in 25m 34.176s of serial time in the second build scan

Not covered

The doFirst block of the GroovyCompile task has been kept as it is a conscious decision

A bunch of tasks having a different build classpath between the 2 executions are still present:

  • FindMainClassTask
  • AsciidoctorTask
  • FetchTagsTask
  • GroovyPageForkCompileTask
  • ExtractDependenciesTask
  • PublishGuideTask
  • CreateReleaseDropDownTask

@jdaugherty
Copy link
Contributor

Thank you @jprinet for your contribution. Several of us are traveling / busy this week. We will try to get to this PR soon.

@jdaugherty
Copy link
Contributor

@jprinet I have not dug into your findings too deep, but I do have a two questions!

  1. It looks like base.dir is coming from the GrailsGradlePlugin and it's used by the old BuildSettings class. I believe the Grails framework uses that to assist in finding the project root to know how/when to enable reloading, where to locate resources, and how to find the grails-app directory. There are fallbacks in the code to not use this property if it's not set (for production run scenarios) and I suspect that's why it's continuing to work (gradle is always run from the same location in the tests). Do you know why this breaks cacheability? I'm wondering if we can find an alternative solution to fix the cacheability.
  2. we have a convention plugin under build-logic for the sbom plugin, does the normalization have to be at the root project levels? Can it go there instead as part of that plugin?

@jprinet
Copy link
Contributor Author

jprinet commented Mar 5, 2026

Just a disclaimer, I am not familiar with the codebase and just looked for a minimal changeset to fix the cache misses 😛

To your question @jdaugherty

  1. this is happening on test tasks, you can identify the 19 culprits there
  2. I will give a try to moving the normalization in there (can't do before next week though)

Related to 2., I also noticed 21 cache misses on AssetForkedCompileTask (coming from asset-pipeline plugin)
They are also due to the sbom.json being different in the Jar files from the classpath / assetClassPath

An opportunistic fix could be to make the assetClassPath input use the RuntimeClasspath normalization instead of a RelativePath one there (mentioning as I found you being the latest contributor 😙)

Fixing the CycloneDxPlugin to generate deterministic sbom.json would probably be a better idea if at all possible

@jdaugherty
Copy link
Contributor

@jprinet thank you, this feedback is incredible. I'm still learning develocity, how can I view the actual values of the properties? Do I need to do this locally or can I see this in develocity itself?

@jprinet
Copy link
Contributor Author

jprinet commented Mar 5, 2026

@jprinet thank you, this feedback is incredible. I'm still learning develocity, how can I view the actual values of the properties? Do I need to do this locally or can I see this in develocity itself?

Develocity does not capture the system properties values by default, but you could add them to a build scan as custom value
The build scan comparison view also has a custom value page, which helps to diff the values between two builds. That would be my recommendation to troubleshoot such case.

Just FYI, this all PR is coming from the results of running the automated validation scripts on the project
This helps to identify non-optimal build cache use on projects, and also comes with a comprehensive summary with pointers to relevant Develocity sections (handy if you are unfamiliar)

@jdaugherty
Copy link
Contributor

Thank you. Do you know if anyone has a github action to make this as part of an established build? Once we fix these, I'd like to have continuous feedback to prevent this in the future =)

@clayburn
Copy link

clayburn commented Mar 5, 2026

We do something like this for Grails here. It uses a set of GitHub Actions you can find here. The idea is to run it weekly or so, and the build will fail if there are cache misses. It would be great if you incorporated the workflow we have into the Grails repository.

@jdaugherty
Copy link
Contributor

Thank you @clayburn I won't have time to look at this until next week, but I'll see what I can do to automate this as part of our build process too then.

@jprinet
Copy link
Contributor Author

jprinet commented Mar 9, 2026

I moved the normalization to the SbomPlugin, 27 additional cache misses appeared compared to the previous version, but the change is way more comprehensive.
You can inspect the cache misses and the build scan comparison @jdaugherty

I believe this solution is still better, although less optimal

@jdaugherty
Copy link
Contributor

FYI: This is still on my list to review. At a high level, the groovy doc & sbom changes are definitely merge-able as-is.

The base.dir issue I need to investigate more because I don't fully understand the impacts. We need to be able to pass that dir to the various tasks or we need to be able to set that some other way - maybe as part of a groovy AST transform. Grails depends on this for it's dev mode to determine reloading support, etc.

I don't have time to debug the gradle side yet though. I need to finish the 8.0 priorities so in all likelihood, I'll have to look at this sometime later this month.

@testlens-app
Copy link

testlens-app bot commented Mar 11, 2026

The TestLens GitHub App is installed in this repository but TestLens is currently in private beta.

Please contact us to finish onboarding this repository.

@jprinet
Copy link
Contributor Author

jprinet commented Mar 11, 2026

I can remove the changes related to the base.dir if you need more time to address this part properly

@jdaugherty
Copy link
Contributor

Let's do that so we can get what you've done merged. I can create a follow-up ticket to track this in the mean time.

@jprinet
Copy link
Contributor Author

jprinet commented Mar 11, 2026

I have just dropped the commit related to base.dir @jdaugherty

@jdaugherty jdaugherty merged commit 45492d4 into apache:7.0.x Mar 11, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants