[ISSUE #1054] Build caching optimizations

[samotleriche]

Fixes #1054.

Motivation

The motivation of this PR is to improve the build time by resolving caching issues.

• build cache was not enabled.
• spotlessApply runs on every build, modifying source code and causing cache misses.
• spotless target included build directory, causing cache misses due to changing inputs.
• WatchFileManagerTest edits a properties file in place causing a cache miss in subsequent tasks.

Modifications

• Enabled gradle build cache by setting org.gradle.caching = true in the gradle.properties file.
• Removed checkstyleMain dependency on spotlessApply (should run spotlessApply as a separate task or integrate into CI).
• Limited the target of spotlessJava to only the src java files.
• Copied configuration.properties into a TempDir for testing the onChange method.

These changes improved the build time for back-to-back builds with no changes from ~4 minutes 30 seconds down to ~15 seconds.

build scan before modifications: https://ge.solutions-team.gradle.com/s/wl6q5fikzjhnq#performance
build scan after modifications: https://ge.solutions-team.gradle.com/s/cssmzbjx5v7js#performance

Documentation

• Does this pull request introduce a new feature? no
• If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented) n/a
• If a feature is not applicable for documentation, explain why? n/a
• If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

[github-actions]

Welcome to the Apache EventMesh community!!
This is your first PR in our project. We're very excited to have you onboard contributing. Your contributions are greatly appreciated!

Please make sure that the changes are covered by tests.
We will be here shortly.
Let us know if you need any help!

Want to get closer to the community?

WeChat Assistant	WeChat Public Account	Slack
		Join Slack Chat

Mailing Lists:

Name	Description	Subscribe	Unsubscribe	Archive
Users	User support and questions mailing list	Subscribe	Unsubscribe	Mail Archives
Development	Development related discussions	Subscribe	Unsubscribe	Mail Archives
Commits	All commits to repositories	Subscribe	Unsubscribe	Mail Archives
Issues	Issues or PRs comments and reviews	Subscribe	Unsubscribe	Mail Archives

[Pil0tXia]

@samotleriche

1. It seems the Build Scan with cache didn't execute tasks: https://ge.solutions-team.gradle.com/s/cssmzbjx5v7js#tests. Can we run tests on top of using build cache?
2. For developers, the build cache is stored on disk. Is it possible to set a frequency for periodically cleaning up expired build caches?

Besides, please link this PR to a corresponding issue~

[pandaapo]

@yanrongzhen Could you please help review the impact of the modification of this PR on the use of Spotless?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1a381ed) 17.39% compared to head (e76ff8d) 17.39%.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #4689   +/-   ##
=========================================
  Coverage     17.39%   17.39%           
  Complexity     1759     1759           
=========================================
  Files           797      797           
  Lines         29850    29850           
  Branches       2579     2579           
=========================================
  Hits           5192     5192           
  Misses        24177    24177           
  Partials        481      481           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[samotleriche]

@Pil0tXia should I link this PR to this issue that was created back in 2022 or create a new issue?
#1054

[Pil0tXia]

@samotleriche It would be great to associate this PR with the issue #1054 you have found.

[samotleriche]

@Pil0tXia the issue has been linked to the PR 🙂

Regarding your two questions:

1. This optimization allows subsequent builds to pull the outputs of a task from cache, rather than re-executing the task if the task's inputs have not changed. The first build runs all the tests and the second build (the one linked which shows no tasks executed) is able to pull the results of the test task from cache.

2. The local build cache is a DirectoryBuildCache, which by default removes unused cache entries after 7 days (ref), but can be configured via the removeUnusedEntriesAfterDays property. Typically the default is fine, but if you need to configure this, it is recommended to configure this via an init script, for the reasons described here.

[Pil0tXia]

able to pull the results of the test task from cache

So, when a class with unit test is modified, the unit test for that class will be automatically executed again, right?

The default value of removeUnusedEntriesAfterDays is 7

I'm referring to that parameter as well. Since it has a default value, there's no need to set it separately.

[samotleriche]

able to pull the results of the test task from cache

So, when a class with unit test is modified, the unit test for that class will be automatically executed again, right?

The default value of removeUnusedEntriesAfterDays is 7

I'm referring to that parameter as well. Since it has a default value, there's no need to set it separately.

Yes, thats correct. Any test tasks that have that class as an input will be re-executed.

[yanrongzhen]

@samotleriche I think maybe we shouldn't change the behavior of modifying source code through spotless during build.
By the way, could you please explain how spotless will affect this pr if it is not modified?

[tylerbertrand]

Hi @yanrongzhen. Jumping in for @samotleriche here - we definitely understand if you don't want to change the current behavior of executing spotlessApply during a build. It is indeed a bit of a process change. Our other optimizations - enabling caching, and limiting spotless to just the src java files - will still have a positive impact on build times.

To quantify the impact of continuing to run spotlessApply on every build... With the improperly formatted code currently committed to master, here is a build scan showing all the tasks that re-run needlessly if we keep the checkstyleMain dependency on spotlessApply. This will happen to some degree any time a developer makes a code change that is not properly formatted and runs a build - spotlessApply will format the code, and on the subsequent build, compilation, tests, etc. will all re-run even if no further changes were made by the developer.

[Pil0tXia]

The changes made after running spotlessJava are not committed to the master branch; instead, they are discarded. We can run spotlessJava separately before each release and commit the formatted changes.

[Pil0tXia]

@samotleriche

Hi! The WatchFileManagerTest unit test failed. May you please help have a check? #4711

[tylerbertrand]

Hey @Pil0tXia, I'll take a look

[Pil0tXia]

@samotleriche @clayburn

Hi, there is a CI failure issue likely caused by enabling Gradle build caching. #4737

May you please help have a check?