Skip to content
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

CI: build once, run on multiple JDKs #454

Merged
merged 8 commits into from
Nov 11, 2024
Merged

Conversation

vhotspur
Copy link
Member

@vhotspur vhotspur commented Nov 5, 2024

Trying how the build times would be changed if we would built Renaissance just once and then pass the pre-built JAR across multiple jobs running it.

@vhotspur
Copy link
Member Author

vhotspur commented Nov 5, 2024

@lbulej You were asking for this, here are some numbers and few comments :-)

If you want to review the changes, I would suggest looking at the current version of the file as the diff is confusing a little bit.

I took the latest run on master and compared it with the latest run on this branch, all numbers are in minutes (and from my experience with CI jobs here, I would say 10% difference is simply fluctuation of the platform).

Jobs master This PR Difference
Everything 187 146 21%
Linux 87 66 24%
MacOS 43 38 11%
Windows 45 35 22%

This will not improve latency as the time required to build and run all the benchmarks varies somewhere between 8 minutes to 17 minutes (depending on JDK, OS, sea temperature in Baltic and whether it is Tuesday in February). But it cuts down the overall CPU-time cost by maybe 20%.

I would say this is definitely improvement for PRs, I wonder if we still do not want to build on JDK11 and latest JDK on master (or perphaps cron these builds for every month or something like that?).

We can probably cut down the time a bit still for the plugins job and maybe split the checks job so that code formatting etc. does not wait for a full build.

As a bonus, we get an artifact from each build -- the GPL-versioned JAR and a JMH JAR. Currently I have set the retention period to the lowest possible value but if we want to keep them around, it can be increased (might make more sense for master only?).

Since the artifacts are at the moment close to 1G, I am not sure how that would play with the free plan.

@lbulej
Copy link
Member

lbulej commented Nov 5, 2024

Thanks a lot! I think it's good.

Sometimes the builds get stuck and fail, so there is less opportunity for things like that. I'm aware that we introduce a dependency on the result of the build task, wherease if everything was parallel, the latency would be hidden. But we never had so many runners, so I think there was some sequential processing where the build added to the total latency in addition to having first built on the latest stable JDK and then on the other/LTS jdks.

The bonus features are welcome, having a longer artifact retention on the master might a good idea, not so much on other/PR branches.

Also, I think if we cron the EA build, it should subsume both building and running on latest release.

@vhotspur vhotspur marked this pull request as ready for review November 7, 2024 09:36
@vhotspur
Copy link
Member Author

vhotspur commented Nov 7, 2024

I did some clean-up and to summarize the changes:

  • Renaissance is built just once, the build is then reused across all other jobs
    • Artifact is kept for 30 days on master, for 4 days on branches/PRs
  • Even the plugins and README-check jobs reuse the prebuilt artifact (the check jobs are now really fast)
  • All tested platforms are using the same matrix
    • I managed to unify steps for Windows, Mac and Linux as they were virtually the same anyway
    • This reduced the CI file size significantly and reduces the chance of omitting some future changes on one of the platforms
  • Individual jobs have lower timeouts (default was 6 hours)
    • Endless loops are detected quicker (they are probably caused by the inherent volatility of the runners running in cloud)
  • Split checks for code formatting away from README checks

Also, I think if we cron the EA build, it should subsume both building and running on latest release.

I would postpone this to a separate PR.

@vhotspur vhotspur requested a review from lbulej November 7, 2024 09:43
Copy link
Member

@lbulej lbulej left a comment

Choose a reason for hiding this comment

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

This looks great to me!

@vhotspur vhotspur merged commit 5358b4e into master Nov 11, 2024
19 checks passed
@lbulej lbulej deleted the topic/gha-caching-builds branch November 11, 2024 12:43
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.

2 participants