-
Notifications
You must be signed in to change notification settings - Fork 1k
Update gradle caching #13208
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
Update gradle caching #13208
Conversation
| - name: Build | ||
| # running suite "none" compiles everything needed by smoke tests without executing any tests | ||
| run: ./gradlew :smoke-tests:test -PsmokeTestSuite=none --no-daemon ${{ inputs.no-build-cache && ' --no-build-cache' || '' }} | ||
| run: ./gradlew :smoke-tests:test -PsmokeTestSuite=none ${{ inputs.no-build-cache && ' --no-build-cache' || '' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--no-daemon was only used in two places, so removed for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the --no-daemon was used here to reduce memory usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when I run with --no-daemon locally it logs
To honour the JVM settings for this build a single-use Daemon process will be forked.
and reading the CodeQL docs they say they need --no-daemon or ./gradlew --stop before running (in order to pick up the initial forking/child process I guess?), which makes me think that --no-daemon doesn't really do much here except prevent it from reusing the existing worker started in "Set up Gradle cache", but I really don't know...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't check the commit history, but I suspect the issue was that the tests sometimes got an OOM. So it was split in 2 parts where in the first part we compile the tests with --no-daemon (-PsmokeTestSuite=none doesn't run any tests) so whatever memory gradle used for building gets freed. Second step just runs the tests and hopefully has gradle consuming less memory. It is possible that the OOM isn't an issue any more. If that is the case we could remove the Build step from that workflow and just keep Test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh! I missed the second step below, that makes sense, I'll add this back
| - name: Build | ||
| # skipping build cache is needed so that all modules will be analyzed | ||
| run: ./gradlew assemble -x javadoc --no-build-cache --no-daemon | ||
| run: ./gradlew assemble -x javadoc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing --no-build-cache should help with CodeQL performance now that it's a PR check
| # gradle enterprise is used for the build cache | ||
| gradle-home-cache-excludes: caches/build-cache-1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing this might help CI performance now that we aren't using gradle enterprise anymore (it should use github actions cache)
No description provided.