Add support for Gradle Configuration Cache

[nedtwigg]

Gradle 6.6 introduces the configuration cache. It looks useful, and it would be great to refactor Spotless to support it. I am okay with bumping our minimum required Gradle in order to support it. gradle/gradle#13490

[nedtwigg]

I took a quick look at this. The first stumbling block is that we use buildFinished:

spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java

Lines 94 to 100 in 41ed32d

	getProject().getGradle().buildFinished(new Closure(null) {
	@SuppressFBWarnings("UMAC_UNCALLABLE_METHOD_OF_ANONYMOUS_CLASS")
	public Object doCall() {
	gitRatchet.close();
	return null;
	}
	});

Googling gradle configuration cache buildFinished surfaces runningcode/gradle-doctor#71, which links to several random commits, of which runningcode/gradle-doctor@120d920 is the most useful. Seems that we can use Gradle::getSharedServices coupled with BuildService to get around this. Probably the BuildService will be a generally-useful part of getting ourselves buildcache friendly...

[nedtwigg]

We're pretty close to being able to ship full support for this. But it's a bit nasty as-implemented, and it could be clean if there were some clarity about how BuildService and ConfigurationCache interact.

• Spotless as it ships today (5.6.1) doesn't interfere with other plugins that already support config cache (huzzah)
• If a Spotless task somehow gets configured, then it breaks config cache for everybody )
  • Fixed 5.15.1 by Configuration cache, part 1 (play nice with others) #720
• To make Spotless actually work with configuration cache is a little more work
  • PR Configuration cache (JVM-local support only, just an idea) #721 implements this, but we're blocked by Improve configuration cache support for Java Object Serialization gradle/gradle#13588, but we could workaround
  • We also use a questionable hack, which I'll describe below

Questionable hack

One of Spotless' features is that you can specify a function in your buildscript, and use that function as a formatter, e.g.:

spotless { format 'blah', {
  custom 'lowercase', { str -> str.toLowerCase() }

However, that lambda is not serializable, so IIUC we'll never be able to support that fully with the traditional configuration cache model of "serialize every task input, execute based on the deserialization of those inputs". However, if there is some kind of ConfigurationCacheService which allows me to take objects that executed during the initial caching, and keep them alive in the daemon for future invocations, then I have a workaround. As it happens, that already exists, by abusing static variables:

• I have this simple little cache in a static variable

• spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java

  Line 35 in 3986fe6

  private Map<String, SpotlessTaskImpl> map = new HashMap<>();

• when configuration actually runs, I store the "original" task in that cache

• spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java

  Lines 78 to 83 in 3986fe6

  	@javax.inject.Inject
  	public SpotlessTaskImpl(FileSystemOperations fileSystemOperations, ObjectFactory objectFactory) {
  	this.fileSystemOperations = fileSystemOperations;
  	this.objectFactory = objectFactory;
  	this.projectDir = getProject().getProjectDir();
  	SpotlessTaskService.instance().put(this);

• and when I'm executing, I grab objects from the original

• spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java

  Lines 51 to 73 in 3986fe6

  	Formatter buildFormatter() {
  	// <sketchy configuration cache trick>
  	SpotlessTaskImpl original = SpotlessTaskService.instance().get(getPath());
  	if (original == this) {
  	// a SpotlessTask is registered with the SpotlessTaskService **only** if configuration ran
  	// so if we're in this block, it means that we were configured
  	return Formatter.builder()
  	.lineEndingsPolicy(lineEndingsPolicy)
  	.encoding(Charset.forName(encoding))
  	.rootDir(getProjectDir().toPath())
  	.steps(steps)
  	.exceptionPolicy(exceptionPolicy)
  	.build();
  	} else {
  	// if we're in this block, it means that configuration did not run, and this
  	// task was deserialized from disk. All of our fields are ".equals" to their
  	// originals, but their transient fields are missing, so we can't actually run
  	// them. Luckily, we saved the task from the original, just so that we could restore
  	// its formatter, whose transient fields are fully populated.
  	return original.buildFormatter();
  	}
  	// </sketchy configuration cache trick>
  	}

This nasty hack works, but it only work if the following holds true:

• when a configuration is cached, the classloader which was used for that caching will be reused for subsequent invocations (holds true right now)
• the classloader will not be used for anything else in-between the caching and subsequent invocations (probably not guaranteed)

IMO, the ability to specify arbitrary functions in a buildscript is a defining value-add of Gradle vs other systems. The configuration cache feature is fantastic, but if I have to give up those arbitrary functions (in this and other plugins), then the caching will always be a second-class feature. To be a first-class, always-works feature, it seems like there needs to be some way to take live objects from the caching, and store them for future invocations.

Interplay between Configuration Cache and BuildService

If you use BuildService as a property on a task (as is recommended), that service gets serialized and deserialized with the task, which is kind of surprising. I'm guessing (but haven't confirmed) that the lifecycle of the service doesn't get managed properly in the configuration cache scenario. The BuildService API is also ambiguous about what counts as "absent" to registerIfAbsent - is it unique per String name, or per String name, Class<T> class, or is the Action<T> init included in there somehow too?

I think it would be a lot clearer, for both BuildService and for the "Configuration Cache Store" that I'm asking for, to have something like this:

public <T> T ObjectFactory.onePerBuild(Class<T> clazz)
public <T> T ObjectFactory.onePerConfigCache(Class<T> clazz)

IMO it's a lot easier to test things when I'm not forced to make them abstract (not a fan of needing to leave methods abstract), and the contract of what Gradle is providing me with is clearer than BuildService.''

Overloaded "configuration"

I get confused by "configuration" the set of dependencies, versus "configuration" the process of executing build scripts and configuring tasks. Also configure is long to type. If I could ask for a breaking change in Gradle, it would be to rename executing build scripts to "setup" - "setup avoidance", "setup-cache". Maybe "wiring" or "init"? Just something unique, and shorter.

[nedtwigg]

@bigdaz are you guys looking for feedback on configuration cache? Super awesome feature, I got it mostly working in Spotless with some hacks, my experience is in the comment above.

[jbduncan]

Oh wow, the code in #721 is going to be... hard to maintain, to put it diplomatically. If there's anything we could do to simplify it, that would be a big win for me. So I'm tempted to wait until we either get some acknowledgement or feedback from the Gradle team or until the configuration cache feature has been developed some more.

In the meantime, I'm happy for #720 to be merged in. On a quick scan, it looks relatively simple.

[nedtwigg]

I agree we should wait for feedback from Gradle folks, and/or more clarity in the docs around it. Merging #720 would make us depend on @Incubating BuildService, and I think it's a not very good API, ObjectFactory.onePerBuild would be better. So if somebody is held-up by Spotless not supporting configuration cache, they can use our jitpack instructions to get either #720 or #721 and then they'll have whatever level of configuration cache support they want. Assuming configuration cache becomes mainstream (I hope it will!), they will both get merged eventually, but there is no rush.

[bigdaz]

I don't have much insight into the Configuration Cache, so I'm not going to be much help. I'll mention @eskatos here since he can likely help.

The best way to have a conversation about this would be on the Gradle Community Slack in the #configuration-cache channel.

[eskatos]

I left a comment about the usage of build services for your use case on #720.

About the formater as a function use case. The user function can be given as an explicit type, an anonymous class, a Java lambda, a Groovy closure, a Kotlin lambda etc... The configuration cache supports serializing some of those already but not all of them in all cases. It shouldn't be a plugin concern though. You can keep your existing API and let users that want to use the configuration cache pass the formater function in a way that is currently supported. Eventually the configuration cache will support more cases.

The nasty hack described above using static state won't fly. The configuration cache can be reused by fresh daemons that won't have the static state initialized.

[nedtwigg]

The configuration cache can be reused by fresh daemons that won't have the static state initialized

Roger. That's the nail in the coffin for our existing design, thanks for the clarification.

It seems like there's a design space of increasing complexity which looks like this:

1. Cache output artifacts from build-to-build on this machine - local build cache
2. Cache output artifacts from build-to-build on any machine - remote build cache
3. Cache configuration from build-to-build on this JVM - JVM-local configuration cache (unplanned)
4. Cache configuration from build-to-build on any JVM on this machine - configuration cache (as it exists)
5. Cache configuration from build-to-build on any machine - remote configuration cache (on the roadmap)

Did you explore option 3 and reject it? It seems like option 3 ought to be faster than 4 or 5 can be, and it's also much easier to implement in some cases because it doesn't require serialization support.

The improvements you guys are making are bringing Gradle's end-to-end runtime out of "build tool" territory and into "fully interactive", which I think is really exciting. The faster Gradle gets, the less important multiple daemons on a single project becomes. Even at its current speed, my usage model almost never requires a second daemon. If it's possible to make "configuration cache" faster for 90% of gradle invocations, at the cost that it only persists across invocations on a single daemon, that seems like a better tradeoff than making it a little slower, with the benefit that it can be serialized to disk and sent to other machines.

Especially given that local & remote buildcache already exist with broad support, it seems like JVM-local caching is the missing hole to allow next-level interactive performance from Gradle. If that hole is ever going to be filled, then I think Spotless will wait for that. And if it's not ever going to be filled, why not?

[eskatos]

Those are very good points. We indeed are considering keeping the cached state warm in the daemon for the next invocation. But we also want to be able to share it between machines. 3/ might not happen soon though.

That being said I don't see why you would be blocked until this happen.

[nedtwigg]

That being said I don't see why you would be blocked until this happen.

You are absolutely right that we can implement something which is compatible with option 4 and 5, and doing so would be nice. But long ago, back in Gradle 2.x, we made the choice to abuse serialization to implement equality. If we want to ignore a field for the purpose of equality (such as a local path), we mark it transient. Definitely a hack, but it has allowed us to build a lot features quickly, without doubling our codesize with .equals and .hashCode

To support option 3 is trivial for us. To support option 4 or 5 is pretty close to a rewrite, and at the end we lose functionality because we're constrained by which things are serializable, and we're a little bit slower than we are with option 3. If Spotless users had option 3 (plus the build cache they already have), I don't think they would mind very much if we didn't support 4 or 5.

• 90%+ of builds can reuse a daemon (as builds get faster, this percentage increases)
• with option 3 the first configure is 3 seconds, and every one after that is 0.2 seconds
• but instead of that, we can rewrite our codebase so that we can stream the configuration cache from the build server, so that every single configure is consistently 0.5 seconds (downside), even the first one (upside, if it happens to be cached)

The configuration cache is a brilliant idea, but it seems like it's being approached backwards, fighting the last battle (buildcache), and long-term restricts what plugins can do. My complaint isn't just that random legacy decisions make it hard in Spotless, it's that my favorite gradle feature is that plugins can accept a function as a build input. I use it all over the place, not just Spotless, it makes my plugins shorter to write, and more powerful and flexible to use.

I love it when other plugins provide function-based APIs too. Configuration cache 3 keeps these as first class citizens, and is faster, and puts a lower cognitive overhead on plugin developers. "Your task inputs stay in memory". If the per-build performance tradeoff of "your task inputs stay in memory" hasn't been compared against the current approach, then I would start there. It's also so simple to implement and document, relative to serializing random lambdas which might capture who knows what. I don't see how that can be done without huge provisos, so I don't see how Gradle doesn't lose "functions as parameters". I don't want "Bazel but with Groovy", I want Gradle!!

[eskatos]

Plugins can still accept functions. It's just that those functions must be serializable. We don't want to lose the convenience. It already works with Groovy closures and Kotlin lambdas. That covers most of the cases. Java lambdas can be problematic though, and this is a problem since forever, not only for the configuration cache, see e.g. gradle/gradle#10751