feat(pkl-gradle): Implement gradle configuration cache support#1500
feat(pkl-gradle): Implement gradle configuration cache support#1500bioball merged 1 commit intoapple:mainfrom
Conversation
0c63abb to
4e4fd66
Compare
4e4fd66 to
7034d01
Compare
7034d01 to
1197bec
Compare
|
Good morning @bioball! Please let me know if there are other requests or questions around getting this verified or merged in. Happy to help out where I can! |
|
Hi! This is on my list of TODOs. I haven't forgotten about it! Apologies, my availability is quite limited right now so I haven't gotten to a review yet. |
|
No problem at all! Appreciate both of your time managing and maintaining the project 😄 |
5240f62 to
70bcc7b
Compare
|
Thanks @bioball! All feedback should be addressed now 😄 |
250be21 to
990ce5a
Compare
Thanks! Feedback incorporated and I squashed to a single commit. |
netvl
left a comment
There was a problem hiding this comment.
Just a couple of questions, overall looks awesome!
Modern versions of Gradle support configuration caching to prevent the gradual increase of project size to affect the overall developer experience of Gradle builds. To prepare the PKL project, and specificall pkl-gradle, for configuration support, we introduce an integration test to vet configuration cache rules, and then perform the necessary updates to provide configuration cache support. Fixes apple#1425
990ce5a to
56b149a
Compare
@netvl thanks for your review! I was able to roll back my null checks, plus some more helpful exception messages on missing sourcesets. |
|
Hey @bioball thanks for merging this! It was great to collaborate to get this update in. Quick question on the release cadence: I know your minor revisions will be frequently deployed Feb/Jun/Oct per your roadmap. Having said that, will there be any plans to cut this change (and the existing merged) as a patch before June? Our team is excited to make use of this enhancement! Thanks again. |
|
We generally don't put behavior changes into patch versions. So, alas, this change will be part of the 0.32 release cycle! |
Fixes #1425
Gradle's configuration cache is one of the more impactful performance features in modern Gradle. It serializes the task graph after the first configuration phase so subsequent builds can skip it entirely. For large projects this can mean the difference between a multi-second configuration overhead and near-instant task scheduling.
The root cause preventing
pkl-gradlefrom adopting the configuration cache was a handful of patterns the configuration cache explicitly prohibits: holding a reference to theProjectobject in a plugin field, accessinggetProject()inside task action code at execution time, and caching CLI option objects that close over non-serializable state. These patterns work fine in a standard Gradle build, but they prevent Gradle from safely serializing the task graph to disk.What changed
PKL Plugin rework
The most visible structural change in
PklPluginis the removal of the@LateInit private Project projectfield. Rather than storing the project on the plugin instance, it's now passed as a parameter through the configure call chain. This is a mechanical but pervasive refactor, since everyconfigure*method gains aprojectargument and passes via property drilling.Per-task changes
BasePklTaskcapturesproject.getLayout().getProjectDirectory()at configuration time into agetWorkingDir()DirectoryProperty, rather than callinggetProject().getProjectDir()at execution time.CliBaseOptionsis now constructed fresh each time it's needed inside a task action, which is a trade-off between the (potentially negligible) cost of construction, and caching it, which would require holding a cross-boundary reference that defeats the configuration cache.EvalTask's lazily-cachedProvider<CliEvaluator>is similarly replaced with a factory method, andJavaCodeGenTaskandKotlinCodeGenTaskleverage the output dir.ModulesTask,getTransitiveModules()changes from aListProperty<File>to aConfigurableFileCollection(enabling.from()wiring rather than.set()), and gains a@PathSensitive(ABSOLUTE)annotation so the task doesn't unnecessarily re-run when files are moved between machines but their content and paths are unchanged. TheparsedSourceModulesCachemap is also removed — it was there to avoid redundant parsing but is incompatible with the cache since it holds mutable state across invocations.Cleanup
The
AnalyzeImportsTaskno longer wrapsCliImportAnalyzerconstruction in a lazily-cachedProvider; instead it constructs the analyzer inline insidedoRunTask(), consistent with the other task types. The transitive-import parsing logic inPklPluginandPluginUtilsis unchanged.Tests
Each task type gets a new
is configuration cache compatibletest that runs the task twice with--configuration-cacheand asserts that the first run stores a cache entry and the second reuses it. The test infrastructure for this lives in a newrunTaskWithConfigurationCachehelper onAbstractTest. NOTE: these tests run slightly slower than the rest of the suite because they need the forked Gradle process to execute twice to prove out configuraion cache usage.