Skip to content

Conversation

@jeantil
Copy link
Contributor

@jeantil jeantil commented Aug 15, 2025

This PR adds build cache support for scala compile and jacoco report-aggregate

According to this build scan (on master) and this build scan (on the initial build cache branch), Scala compilation and jacoco report aggregation are the 2 longest running goals in the build when it is otherwise fully cached.

Note that this PR is WIP : local runs seem to be able to store to the build cache. -DrerunGoals ensures that all cache output is stored again in case some cache pollution occured.

mvn -e clean test -P ci-test  -DskipTests -Dassembly.skipAssembly=true jacoco:report-aggregate@jacoco-report -pl :mailetdocs-maven-plugin -am -DrerunGoals 

but running again without the rerunGoals

mvn -e clean test -P ci-test  -DskipTests -Dassembly.skipAssembly=true jacoco:report-aggregate@jacoco-report -pl :mailetdocs-maven-plugin -am

fails to compile on my machine with

[INFO] --- scala:4.9.5:compile (scala-compile-first) @ event-sourcing-core ---
[INFO] compiler plugin: BasicArtifact(org.scalameta,semanticdb-scalac_2.13.16,4.9.6,null)
[DEBUG] Local cache hit (29 KB)
[INFO] Loaded from the build cache, saving 2.305s
[INFO]
[INFO] --- compiler:3.14.0:compile (default-compile) @ event-sourcing-core ---
[DEBUG] Local cache miss
[DEBUG] Remote cache miss
[INFO] Recompiling the module because of changed dependency.
[INFO] Compiling 1 source file with javac [debug parameters release 21] to target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /home/jean/dev/secretjames/master/event-sourcing/event-sourcing-core/src/main/java/org/apache/james/eventsourcing/CommandHandler.java:[25,43] cannot find symbol
  symbol: class Command
[ERROR] /home/jean/dev/secretjames/master/event-sourcing/event-sourcing-core/src/main/java/org/apache/james/eventsourcing/CommandHandler.java:[28,18] cannot find symbol
  symbol:   class EventWithState
  location: interface org.apache.james.eventsourcing.CommandHandler<C>
[INFO] 2 errors

I'm not sure why there is a cache miss on the java compilation so this it not yet mergeable

links I found helpful working on this (in case I accidentally close my browser tabs :D)

  • documentation on making unsupported goals cacheable
  • examples of custom goal caching definitions
  • source code for scala maven plugin
  • source code for jacoco plugin which should be supported by default but somehow isn't.
  • using MAVEN_OPTS=-Dorg.slf4j.simpleLogger.log.develocity.goal.cache=debug show some useful information on goal caching

<ignoreEmptyDirectories>true</ignoreEmptyDirectories>
</normalization>
</fileSet>
<fileSet>
Copy link
Contributor Author

@jeantil jeantil Aug 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using the classpathElements as items in the cache key solves the invalid cache items so compilation works
locally for executions in rapid succession the local cache works but on the ci with 5h between jobs we get cache misses with out of order items
classpathElements is a set in the scala compiler plugin and caching it was inspired by the kotlin example and the clojure example

the cause is probably that the scala maven plugin creates a new set with

   @Override
  protected Set<File> getClasspathElements() throws Exception {
    Set<File> back = FileUtils.fromStrings(project.getTestClasspathElements());
    back.remove(new File(project.getBuild().getTestOutputDirectory()));
    addAdditionalDependencies(back);
    return back;
  }

whereas the clojure plugin uses a stored proeprty

  @Parameter(required = true, readonly = true, property = "project.compileClasspathElements")
  protected List<String> classpathElements;

the additional dependencies was added as a way to introduce compile-only dependencies to generate better documentation

not sure how to fix that, I'm trying to push a patch upstream

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the patch was accepted upstream and released yesterday I have updated the PR to resume my tests

@jeantil
Copy link
Contributor Author

jeantil commented Sep 13, 2025

with build cache enabled for scala compilation, a rebuild on an unchanged james codebase unsing mvn clean test-compile goes from

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  05:12 min
[INFO] Finished at: 2025-09-13T08:59:36+02:00
[INFO] ------------------------------------------------------------------------
[INFO] 3145 goals, 3145 executed

down to

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  45.014 s
[INFO] Finished at: 2025-09-13T08:52:55+02:00
[INFO] ------------------------------------------------------------------------
[INFO] 3145 goals, 2290 executed, 855 from cache, saving at least 4m 12s
[INFO]
[INFO] Publishing build scan...

@jeantil jeantil force-pushed the improve-build-cache branch 3 times, most recently from ea20a0d to fd07344 Compare September 14, 2025 13:41
@jeantil jeantil marked this pull request as ready for review September 14, 2025 14:10
<normalization>
<runtimeClassPath>
<ignoredFiles>
<ignoredFile>git.properties</ignoredFile>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't been able to retrieve the contents to understand why they were different but build scan comparison showed that this file was preventing caching of the test outputs

@jeantil jeantil force-pushed the improve-build-cache branch from fd07344 to 7de3d41 Compare September 14, 2025 14:25
using runtimeClasspath normalization to ignore git.properties
which was pointed out as preventing caching by a build scan
comparison
git.properties are not expected to change test and coverage output much
anyways.
@jeantil jeantil force-pushed the improve-build-cache branch from 7de3d41 to 9d8cfeb Compare September 14, 2025 14:27
@jeantil
Copy link
Contributor Author

jeantil commented Sep 14, 2025

This build shows test output successfully loaded from the build cache.
The next build I expect to last 4-5h and to store all the test outputs in a reusable manner.
The following build lasted 19 minutes total.

If build caching proves problematic, disabling scala compilation caching can easily be achived and the build time cost of not caching compilation is not that high (a few minutes)
I am very confident that the runtimeClasspath normalization is safe
Even if I got the jacoco coverage report wrong it shouldn't be a blocker. At worse the coverage reports will be inaccurate.

Once merged on master, I expect the develocity performance report for the test stage to show a massive build time gain

@jeantil jeantil changed the title Improves james build cache [JAMES-3978] Improves james build cache Sep 14, 2025
@jeantil jeantil merged commit ed60d9b into apache:master Sep 14, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant