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

Configuration cache (JVM-local support only, just an idea) #721

Closed
wants to merge 6 commits into from

Conversation

nedtwigg
Copy link
Member

This PR builds on top of #720, to build full support for the configuration cache. The big thing left is that everything in Spotless (namely the git line-endings policy and all the FormatterStep) need to become round-trip serializable, with custom readObject / writeObject methods.

In many places throughout Spotless, we use a trick where if two objects serialize to the same bytes, then they are equal. To make that work, almost everything in Spotless is serializable, but there are lots of transient fields, which are not correctly restored when an object is deserialized. I can think of two solutions:

  1. Remove all the transient fields all over the place, and implement equality "properly", rather than cheating and using a "serializes to the same bytes" hack.
  2. Create a special BuildService which persists across the lifetime of a given configuration-cache, and use that to "rehydrate" fields which are not correctly deserialized.

Solution 2 is a lot easier for us, and it also has the benefit that it would support Spotless' full suite of features. With solution 1, we will never be able to support closures which are in-line within a buildscript. That would be a real shame, because Gradle's inline closures are such a great part of the tool, and every plugin (not just Spotless) will be unable to use such closures unless there is some kind of long-lived BuildService.

… place where this is handled to be config-cache friendly.
…ed to reference the "worker" task `spotlessFoo` to do various things. We create a `SpotlessTaskService implements BuildService` to serve this role, which allows configuration cache to complete without errors.
…potlessTask from a previous build (the last build which actually configured)
…serialization error:

```
Caused by: java.lang.InstantiationError: [B
	at org.gradle.instantexecution.serialization.beans.BeanPropertyReader.newBean(BeanPropertyReader.kt:58)
	at org.gradle.instantexecution.serialization.beans.BeanStateReader$DefaultImpls.newBeanWithId(BeanStateReader.kt:25)
	at org.gradle.instantexecution.serialization.beans.BeanPropertyReader.newBeanWithId(BeanPropertyReader.kt:37)
```
@nedtwigg
Copy link
Member Author

nedtwigg commented Oct 16, 2020

The commit two above this implements Solution 2 using a private static field which gets persisted between buildcache runs. It shows that we could use this approach to add fill buildcache support pretty easily (CI passed everywhere except windows).

The commit one above this shows that we still have serialization problems, probably related to gradle/gradle#13588

@nedtwigg
Copy link
Member Author

TODO: If a git branch changes location during a string of configuration cache runs, we need to update this value. Should be pretty easy to create a GitReference object which updates the SHA from the reference string on deserialization.

@Input
public ObjectId getRatchetSha() {
return subtreeSha;
}

@nedtwigg nedtwigg closed this Nov 7, 2021
Copy link
Member Author

@nedtwigg nedtwigg left a comment

Choose a reason for hiding this comment

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

I think this JVM-local thing is still a good idea that we should go with for now.

Comment on lines +51 to +73
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>
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +37 to +43
synchronized void put(SpotlessTaskImpl task) {
map.put(task.getPath(), task);
}

synchronized SpotlessTaskImpl get(String taskPath) {
return Objects.requireNonNull(map.get(taskPath), taskPath);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

@nedtwigg nedtwigg reopened this Nov 8, 2021
@nedtwigg nedtwigg changed the title Configuration cache, part 2 (full support) Configuration cache (JVM-local support only, just an idea) Nov 8, 2021
@nedtwigg
Copy link
Member Author

nedtwigg commented Nov 8, 2021

#986 implements this idea but implemented better.

@nedtwigg nedtwigg closed this Nov 8, 2021
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.

1 participant