SOLR-15603: Activate Gradle build cache #277

alextu · 2021-08-30T14:22:52Z

https://issues.apache.org/jira/browse/SOLR-15603

Description

Activate gradle local build cache to take advantage of re-using past build executions on a single machine, it improves build time when switching between branches for example.

Solution

Set the gradle build cache flag, upgrade gradle to 6.9.1, upgrade a 3rd party plugin and make some tasks cacheable when easily doable. More context below:

the most costly task is solr:core:test, it takes advantage of up-to-date checks and local cache if the tests.seedproperty is set to a fixed value. That said it does not take advantage of remote cache because some system properties use absolute path.
Using a remote cache would be interesting since you have a CI that could publish to a remote cache node, remote developpers would then have faster local builds.
checkBrokenLinks, renderJavadoc, renderSiteJavadoc, buildLocalJavadocLinksSite, buildSite, prepareLocalJavadocLinksSiteSources, prepareSiteSources tasks are now cacheable, but still :solr:solr-ref-guide:jrubyPrepare is always executed and quite heavy (dowloading/installing gems).

Tests

Manually checked the build performance improvement:

./gradlew clean check -Ptests.seed=DEADBEE
BUILD SUCCESSFUL in 27m 26s
409 actionable tasks: 351 executed, 13 from cache, 45 up-to-date

./gradlew clean check -Ptests.seed=DEADBEE
BUILD SUCCESSFUL in 45s
409 actionable tasks: 260 executed, 99 from cache, 50 up-to-date

Checklist

Please review the following and check all that apply:

I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
I have created a Jira issue and added the issue ID to my pull request title.
I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
I have developed this patch against the main branch.
I have run ./gradlew check.
I have added tests for my changes.
I have added documentation for the Reference Guide

dweiss · 2021-08-30T14:33:41Z

build.gradle

@@ -20,7 +20,7 @@ import java.time.format.DateTimeFormatter

 plugins {
  id "base"
-  id "com.palantir.consistent-versions" version "1.14.0"


There is a patch upgrading this plugin (and all of gradle infrastructure) here - #275. It'll conflict with your PR, once applied. I'll cherry pick relevant areas if you don't want to spend any more time on it.

Ho, I missed this effort upgrading to 7.x, good stuff ! I can wait for your branch to be merged and I'll fix my branch

I'm planning to merge tomorrow, sorry for not mentioning this earlier.

dweiss · 2021-08-30T14:35:33Z

gradle/generate-defaults.gradle

@@ -42,6 +42,7 @@ configure(rootProject) {
                "systemProp.file.encoding=UTF-8",
                "org.gradle.jvmargs=-Xmx3g", // TODO figure out why "gradlew check" runs out of memory if 2g
                "org.gradle.parallel=true",
+                "org.gradle.caching=true",


This enables local caches, right? I'm not sure folks will be enthusiastic about their home folders increasing in size so dramatically - solr build outputs are pretty heavy.

Right, so if you're afraid of this, I can remove this setting and you can enable it through the command line when building, check after a few weeks of dev if it's acceptable. It's hard to predict the size, currently I see on my machine it's about 400Mb (after removing the cache directory and running a few solr builds)

I think this should be left commented out in the generated user defaults - let it be an opt-in. I plan to commit gradle 7.2 path tomorrow and reiterate on your patch, cherry picking the obvious parts of it. Stay tuned. :)

dweiss · 2021-08-30T14:36:18Z

gradle/validation/check-broken-links.gradle

@@ -40,11 +41,13 @@ class CheckBrokenLinksTask extends DefaultTask {
  @InputFile
  File script = project.rootProject.file("dev-tools/scripts/checkJavadocLinks.py")

+  @OutputFile


Yup, this is good, thanks.

dweiss · 2021-08-30T14:36:55Z

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-6.8.3-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-6.9.1-bin.zip


this alone won't work - requires a new checksum too (dont in that other PR).

solr/solr-ref-guide/build.gradle

dweiss · 2021-08-30T14:37:50Z

solr/solr-ref-guide/build.gradle

 abstract class PrepareSources extends DefaultTask {
    // Original Source files we'll be syncing <b>FROM</b>
    @InputDirectory
-    public DirectoryProperty srcDir = project.objects.directoryProperty()
+    abstract DirectoryProperty getSrcDir()


Are these abstract properties automatically injected if you declare them like this? Where is the gradle docs that explains this (and which properties can be injected like so)? Thanks.

Yes exactly, so the docs are here but afaik it does not explain the runtime magical injection happening.

Ah, I see it now. Yeah - it'd be good to require the Inject annotation on those abstract properties too. This would be more intuitive (to me at least).

dweiss · 2021-09-01T09:03:24Z

Hi Alexis. I've rebased your patch on top of main (gradle 7.2) and cleaned up some minor noise here and there. The build currently fails though - not sure which normalization strategy would be suitable here.

A problem was found with the configuration of task ':solr:solrj:renderSiteJavadoc' (type 'RenderJavadocTask').
  - Type 'RenderJavadocTask' property 'srcDirSet' is annotated with @InputFiles but missing a normalization strategy.

alextu · 2021-09-01T09:41:33Z

Hi Dawid, I will rebase locally and investigate (today hopefuly)

dweiss · 2021-09-01T12:39:17Z

Hey, no rush. I don't think you need to rebase - just pull the changes? I pushed the cleanups directly to your branch (PRs allow for this).

alextu · 2021-09-01T12:41:59Z

Ho right, I missed it, after rebasing I've managed to fix those issues locally, is it ok for you if I force push my changes with a clean commit ?

dweiss · 2021-09-01T13:30:27Z

Sure, go ahead.

checkBrokenLinks, renderJavadoc, renderSiteJavadoc, buildLocalJavadocLinksSite, buildSite, prepareLocalJavadocLinksSiteSources, prepareSiteSources tasks are now cacheable. To take real advantage of the cache between builds, the seed property should be fixed, for example: ./gradlew clean check -Ptests.seed=DEADBEE

alextu · 2021-09-01T20:04:33Z

@dweiss ok it's pushed, it should be fine now

…efault.

…cleanups (apache#277)

dweiss reviewed Aug 30, 2021

View reviewed changes

alextu force-pushed the gradle-build-imprv branch from 47647f3 to 8b60cf5 Compare September 1, 2021 20:03

dweiss added 3 commits September 1, 2021 23:00

Add a note about org.gradle.caching and disable the cache option by d…

0a79d8e

…efault.

Changes entry.

a861c9d

Merge remote-tracking branch 'origin/main' into gradle-build-imprv

0b48fc7

dweiss merged commit c3bef8c into apache:main Sep 6, 2021

epugh pushed a commit to epugh/solr that referenced this pull request Oct 22, 2021

SOLR-15603: Add an option to activate Gradle build cache, build task …

15978e2

…cleanups (apache#277)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SOLR-15603: Activate Gradle build cache #277

SOLR-15603: Activate Gradle build cache #277

alextu commented Aug 30, 2021

dweiss Aug 30, 2021

alextu Aug 30, 2021

dweiss Aug 31, 2021

dweiss Aug 30, 2021

alextu Aug 31, 2021

dweiss Aug 31, 2021

dweiss Aug 30, 2021

dweiss Aug 30, 2021

dweiss Aug 30, 2021

alextu Aug 30, 2021

dweiss Aug 31, 2021

dweiss commented Sep 1, 2021

alextu commented Sep 1, 2021

dweiss commented Sep 1, 2021

alextu commented Sep 1, 2021

dweiss commented Sep 1, 2021

alextu commented Sep 1, 2021

SOLR-15603: Activate Gradle build cache #277

SOLR-15603: Activate Gradle build cache #277

Conversation

alextu commented Aug 30, 2021

Description

Solution

Tests

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dweiss commented Sep 1, 2021

alextu commented Sep 1, 2021

dweiss commented Sep 1, 2021

alextu commented Sep 1, 2021

dweiss commented Sep 1, 2021

alextu commented Sep 1, 2021