spotless gradle plugin leaks memory (massively)

[lkoe]

From my preliminary analysis of instabilities in our build environment I suspect that spotless gradle plugin is massively leaking memory in the gradle daemon.

It seems that via GitAttributesLinieEndings$RelocatablePolicy the JVMLocalCache holds onto complete project references, which cannot be collected by a full gc of the daemon process.

The image shows a full gc after the build of one of our projects (admittedly one having about 35 subprojects), with spotless applied vs. spotless not applied.
Also shown a GC root analysis on the daemon heap dump, showing the suspected offender.

The expectation would be that spotless only caches calculated values (and using weak cache values that could be collected if need be).

Using spotless-gradle-plugin 6.5.2

Gradle 7.3.3
------------------------------------------------------------

Build time:   2021-12-22 12:37:54 UTC
Revision:     6f556c80f945dc54b50e0be633da6c62dbe8dc71

Kotlin:       1.5.31
Groovy:       3.0.9
Ant:          Apache Ant(TM) version 1.10.11 compiled on July 10 2021
JVM:          11.0.12 (Amazon.com Inc. 11.0.12+7-LTS)
OS:           Windows 10 10.0 amd64

[nedtwigg]

Thanks for the bug report, I should have a fix published by tomorrow.

[lkoe]

Thanks, much appreciated.
I tried to work around by setting spotless.lineEndings=PLATFORM_SPECIFIC (from memory).
This "solved" this particular occurrence, but revealed more offenders. Couldn't finish this today, but will attach more info tommorow morning (CEST).

[lkoe]

Ok, I after dealing with lineEndings as above, the next issue seems to be GradlePrivsioner#forConfigurationContainer , where project related references are "baked" into a lambda, which cannot be collected:

This affects RemoveUnusedImportsStep, GsonStep and EclipseBasedStepBuilder.

After disabling those from my spotless config, another issue is shown with LicenseHeaderStep, which is potentially induced by https://github.com/diffplug/spotless/blob/main/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java#L505 (again a lamda using a project related ref?)

After disabling the LicenseHeaderStep from our config, full GC could finally reclaim the memory in the daemon process again 😅

So it appears the issue is a bit more wide spread than initially suggested, hinting to a bit of a conceptual "tug-of-war" between caching and lazyness.

[lkoe]

Thanks for the fix, I tested it.
It works, if all projects in a build are successfull.

Unfortunately, if one of the projects in a multi-build fails (e.g. during spotlessCheck), RelocatablePolicy#calculateState is never called for the remaining projects, so in this case the leak remains.
I haven't tested if the other fix locations would trigger a similar problem, but looking at the code, they might

[nedtwigg]

Thanks for testing this, the commit above makes sure that calculateState is always called before a value makes it into the daemon-level cache.

[lkoe]

Thanks, re-tested, unfortunately there seems to be still a reference that slips by in the "broken build" scenario, coming from GradleProvisioner.

JVisualVM seems to pick one of the uncollectable reference chains when displaying GC roots, so they were hidden by the earlier issue. Sorry for the "drop by drop" reporting...

[nedtwigg]

Sorry for the "drop by drop" reporting...

Are you kidding!? Thanks for sticking with this! I'm giving drop-by-drop fixes, and I think our incremental approach is the fastest way we could do this. The code design has definitely picked up some warts as the Gradle model has changed, but the lazyness is still helpful to how Spotless is used in Maven builds, so I think the little kludges we've had to accept are our best option.

[nedtwigg]

I think this is fixed in plugin-gradle 6.6.0. Can you confirm before I close this?

[lkoe]

Thanks again, I tested the 6.6.0 release but still see the leak (in the "broken build" scenario) via the same reference chain as above (#1194 (comment))

~~ As fair as I understand the code, the latest changes do not yet apply there, bc. RemoveUnusedImportsStep and GsonStep (and possible others) are creating FormatterStepImpl via

spotless/lib/src/main/java/com/diffplug/spotless/java/RemoveUnusedImportsStep.java

Line 32 in 49099f3

return FormatterStep.createLazy(NAME,

and https://github.com/diffplug/spotless/blob/main/lib/src/main/java/com/diffplug/spotless/FormatterStep.java#L107 which in itself is not yet a DelegateFormatterStep as per the lastest commit. ~~

[lkoe]

Pls disregard my earlier analysis attempt - I see that FormatterStepImpl already extends LazyForwardingEquality, but it appears it is not working in all cases.

[lkoe]

Ok, did some more debugging and it appears that value here (https://github.com/diffplug/spotless/blob/main/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java#L57) can be a collection of FormatterStepImpl in which case the "unlazying" doesn't happen

[lkoe]

After some local patching it seems that losing the instanceof and the cast to LazyForwardingEquality here (https://github.com/diffplug/spotless/blob/main/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLocalCache.java#L57) does indeed fix all remaining leaks, as LazyForwardingEquality#unlazy already takes care of all the corner cases (delegates, collections).

	static class LiveCacheKeyImpl<T> implements LiveCache<T>, Serializable {
                ...
		@Override
		public void set(T value) {

			// whenever we cache an instance of LazyForwardingEquality, we want to make sure that we give it
			// a chance to null-out its initialization lambda (see https://github.com/diffplug/spotless/issues/1194#issuecomment-1120744842)
			LazyForwardingEquality.unlazy(value);
			daemonState.put(internalKey, value);
		}

I couldn't reproduce any leaks anymore after applying that change

[lkoe]

Hmm, while the "unlazying" cleans up the memory leaks, I have now a new problem at hand to deal with.

We wrap spotless-gradle (among others) in our own inhouse convention plugin, which configures all of our projects consistently.
In order to achieve this, our plugin "smuggles" a custom task before the spotless tasks to create the necessary file structure in the project dynamically. These files are located within our plugin and are essentially copied from the plugin classpath into the projects.

However, now with the fixes above in place - spotless tries to resolve all its configuration file (headers, import order...) earlier than before, even before our custom task had a chance to export all those files into the project.

The stacktrace looks like this:

com.diffplug.spotless.ThrowingEx$WrappedAsRuntimeException: java.nio.file.NoSuchFileException: <path-tp-project>\build\formatter\properties\licence.header
        at com.diffplug.spotless.ThrowingEx.asRuntime(ThrowingEx.java:92)
        at com.diffplug.spotless.LazyForwardingEquality.state(LazyForwardingEquality.java:58)
        at com.diffplug.spotless.LazyForwardingEquality.unlazy(LazyForwardingEquality.java:118)
        at com.diffplug.spotless.LazyForwardingEquality.unlazy(LazyForwardingEquality.java:124)
        at com.diffplug.gradle.spotless.JvmLocalCache$LiveCacheKeyImpl.set(JvmLocalCache.java:61)
        at com.diffplug.gradle.spotless.SpotlessTask.setSteps(SpotlessTask.java:168)
        at com.diffplug.gradle.spotless.SpotlessTaskImpl_Decorated.setSteps(Unknown Source)
        at com.diffplug.gradle.spotless.FormatExtension.setupTask(FormatExtension.java:762)
        at com.diffplug.gradle.spotless.SpotlessExtensionImpl.lambda$createFormatTasks$6(SpotlessExtensionImpl.java:70)
        ...
        at com.sun.proxy.$Proxy56.afterEvaluate(Unknown Source)

So it looks like this all happens in afterEvaluate now.

This means I could now:

• drop our custom spotless profile export task and invoke its logic during configuration (not ideal)
• ... ?

A better solution (in our use case) would be if I could configure the spotless extension directly with classpath resources, eliminating the need to export the spotless profile into each project directory altogether.
What's your take on this?
If you consider this worthwhile I could try to whip up a PR, which at least shows the intention.

[nedtwigg]

Yeah, a consequence of fixing the memory leaks is that the FormatterStep get configured sooner than they did before, so thanks for posting about that here. I'm going to close this issue because the memory leak is solved, and I'm going to open a new issue about loading resources from the classpath (and other places) and timing issues therein.

[nedtwigg]

Here's the issue re: loading resources from the classpath.

• Load config resources from classpath / URL #1204

[lkoe]

I'm sorry, but would you mind to still apply the final fix (or something similar) suggested here? #1194 (comment)

Edit: added a PR for that #1206

[lkoe]

Thank you, fix confirmed - I don't see any leaks anymore 👍

[ZacSweers]

I think this issue has regressed in later versions. We are seeing a huge amount of memory being taken by Spotless JvmLocalCache in 6.18.0, even when not running spotless itself.

[nedtwigg]

1. There are a few low-hanging fruit I need to get to which will reduce the scope of our configuration cache workaround
2. From the attached screenshot, I can't see the problem. Earlier in the thread, there are screenshots which show how an unwanted class is being kept by some GC root. In the screenshot above, all I can see is that JvmLocalCache is consuming 1.3 million somethings - bytes? Which seems modest. I'm guessing I'm misreading the screengrab.

Happy to reopen and squash this if you have time to trace GC roots, or if you want to wait for the issues below to close first that works too

• Spotless Gradle Plugin fails Gradle 8.1 with configuration cache enabled #1644
• obsolete - Internal-only breaking changes #1096

[lkoe]

Hi there, while upgrading to the latest spotless I revisited the issue and indeed it seems to have regressed.

Here are some GC roots holding project references, though I didn't quite wrap my head around whats happening.
Common denominator seems to be FormatExtension holding onto a project ref via lamda:

Edit: spotless-plugin-gradle-6.21.0 on gradle-8.3

[lkoe]

bump

[denzap]

Have the same problem
Seems like this is regression bug since 6.21.0

[nedtwigg]

We have started the process of removing JvmLocalCache and supporting Gradle configuration cache without any tricks

• Support Gradle configuration cache on multiple JVMs  #1274

It will take a while for this effort to fully complete, but there is progress on the horizon here.