MergeDiagnosticJson should not be build cacheable #1436
Conversation
Generate changelog in
|
| @@ -16,7 +16,4 @@ | |||
|
|
|||
| package com.palantir.gradle.dist.service; | |||
|
|
|||
| import org.gradle.api.tasks.CacheableTask; | |||
|
|
|||
| @CacheableTask | |||
| public abstract class MergeDiagnosticsJsonTask extends MergeDiagnosticsJsonTaskImpl {} | |||
There was a problem hiding this comment.
Might be worth a comment so we don't attempt to re-"optimize" this in the future!
There was a problem hiding this comment.
hmm, I think in that case we would end up putting that comment on every single simple task we write...
There was a problem hiding this comment.
you can go even better: @DisableCachingByDefault(because = "reasons"), you see it on the Copy task.
There was a problem hiding this comment.
I don't think it would be entirely unreasonable to explicitly declare whether each task is cacheable or not, but I defer to your judgement.
…alantir/sls-packaging into callumr/merge-diagnostic-json-no-cache
|
Released 7.29.0 |
Before this PR
mergeDiagnosticJsonis cacheable, meaning that Gradle will spend 5 seconds looking for a cache entry to save doing ~10-20 milliseconds of actual work (given the jars it's scanning through is the runtime classpath, which is always needed and already cached). This is quite a common sight:After this PR
==COMMIT_MSG==
mergeDiagnosticJsonis no longer build cacheable, as it spends 100x more time accessing the cache than just running the task==COMMIT_MSG==
Possible downsides?