casid / jte

Secure and speedy templates for Java and Kotlin.
https://jte.gg
Apache License 2.0
748 stars 56 forks source link

CI: Speeding up jobs #296

Closed marcospereira closed 9 months ago

marcospereira commented 10 months ago

What?

A few changes to speed up the CI execution:

Required changes

Results

Here is a run with no cache: https://github.com/marcospereira/jte/actions/runs/6803428787/job/18498736885. The "Build with Maven" step took almost 3 minutes.

Here is the same commit, but running with a cache: https://github.com/marcospereira/jte/actions/runs/6803428787/job/18499856271. The "Build with Maven" step took 11 seconds.

This is not the most realistic scenario because there will be code changes preventing the build cache from being fully reutilized, but it shows the possible gains.

Decisions

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (827e4e7) 91.15% compared to head (aa33348) 91.15%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #296 +/- ## ========================================= Coverage 91.15% 91.15% Complexity 1196 1196 ========================================= Files 76 76 Lines 3132 3132 Branches 484 484 ========================================= Hits 2855 2855 Misses 168 168 Partials 109 109 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

casid commented 10 months ago

@marcospereira thanks!

Caching Gradle and Maven dependencies is fantastic, this alone should speed up CI quite a bit.

However, I'm very sceptical regarding maven-build-cache-extension. Caching is a hard problem and I'm wondering if maven is really always able to do the correct thing here (for instance, when we change CI parameters that maven is not aware of). I'd feel better if we would always do a dead simple clean build.

The main problem of slow builds are the ridiculously slow Kotlin compile times, but I haven't found a way to speed those up so far.

marcospereira commented 10 months ago

However, I'm very sceptical regarding maven-build-cache-extension. Caching is a hard problem and I'm wondering if maven is really always able to do the correct thing here (for instance, when we change CI parameters that maven is not aware of). I'd feel better if we would always do a dead simple clean build.

I understand the sentiment, and that is why I made it an opt-in that only runs in the CI, without interfering in local env unless the cache is manually enabled. But I'm leaning towards its benefits are bigger than the risks.

Also, I cannot think of a good example of CI configuration requiring a cache eviction. One thing we can do, though, is to have the workflow YAML files being part of the cache key (including them in the hashFiles here) so that there will be a cache miss when those files change.

Makes sense?

marcospereira commented 10 months ago

The main problem of slow builds are the ridiculously slow Kotlin compile times, but I haven't found a way to speed those up so far.

I also noticed that and plan to investigate it when I have the spare cycles. I suspect using Kotlin daemon (not currently used by jte) would be helpful not only for tests but for developer experience too.

casid commented 9 months ago

I understand the sentiment, and that is why I made it an opt-in that only runs in the CI

IMHO, ideally CI should always be the source of truth, thus it should always create a fresh, clean build if possible.

I suspect using Kotlin daemon (not currently used by jte) would be helpful not only for tests but for developer experience too.

I don't think this will help. During development (aka a server is running), jte + the Kotlin compiler used by jte are part of the already running server process. I think it won't get any faster than passing the templates to compile directly to the embedded Kotlin compiler. The problem isn't that we don't use a daemon. If I compare with Java compile times, which does not use a daemon and is almost an order of magnitude faster.

marcospereira commented 9 months ago

@casid, this is now rebased on top of main branch, and I removed the maven build cache. The pr description is also updated with such changes.

casid commented 9 months ago

Thank you so much @marcospereira!