SpotlessApply uses by default the Java formatter by crogoz · Pull Request #1243 · palantir/palantir-java-format · GitHub

crogoz · 2025-03-19T08:57:36Z

Before this PR

The throughput for formatting big repos is decreased by using the native image (CE edition) - see internal metrics doc here and comment here
java-based implementation 32 secs vs native-image implementation 1min 20sec.

Even though spotlessApply is incremental and cacheable, in case the task cannot run incrementally, the performance is not good.

After this PR

When running spotlessApply tasks only (to format full projects), we will switch to using:

Java-based implementation if the daemonTarget already is set to Java 21
otherwise we will use the native-image until the rollout of Java 21 is finished everywhere.

formatDiff/Intellij will still use the native image formatter by default - the perf for running on a few files is better with native-image vs Java-based implementations.

==COMMIT_MSG==
SpotlessApply uses by default the Java formatter
==COMMIT_MSG==

Possible downsides?

changelog-app · 2025-03-19T08:57:40Z

Generate changelog in `changelog/@unreleased`

What do the change types mean?

feature: A new feature of the service.
improvement: An incremental improvement in the functionality or operation of the service.
fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
and external consumers of the service's API (e.g. customer-written software or integrations).
deprecation: Advertises the intention to remove service functionality without any change to the
operation of the service itself.
manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
performing database surgery, ...) at the time of upgrade for it to succeed.
migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

❗The break and manual task changelog types will result in a major release!
🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
✨ All others will result in a minor version release.

Type

Description

SpotlessApply uses Java 21 version

Check the box to generate changelog(s)

Generate changelog entry

crogoz · 2025-03-19T08:59:38Z

+
+    }
+
+    private static Iterable<File> getBuildPluginClasspathInjector() {


should I move this to https://github.com/palantir/gradle-plugin-testing ? gradle-jdks needs to do the same thing in order to run on multiple Java versions.

I don't understand what it's supposed to do?

I suppose what I don't understand is why you need this at all? I thought nebula does this for you?

I need to test how the task works against Java 17 and Java 21. So, I am using the gradle-jdks automanagement workflow to set-up the JDKs but then I also need to run ./gradlew instead of the classic nebula runTask . When using nebula runTask the plugin is injected automatically, but now I need to set it on my own. This is what getBuildPluginClasspathInjector does - it adds the files of the plugin to the test classpath.

I have the same setup in gradle-jdks for tests - so I was wondering if it makes sense to move this logic in the testing plugin.

oh, I thought the tooling api invoked gradlew, interesting!

you could move it to gradle-plugin-testing, it's a bit niche, but I don't see why not.

I think a nicer way to do this, is probably to reuse some of the stuff in Nebula, it makes your life a lot easier I think:

Use ClasspathAddingInitScriptBuilder to write the classpath in an init script form

include the init script when running gradlew. i.e. ./gradlew <task> -i init-script

I think the current approach - I also mentioned something similar in the testing plugin PRs - detracts from the test by adding a bunch of cruft that's not actually important to know for the test, but is unfortunately required for the test to pass.

It also means, that you can hide all of this behind a runGradlew method and not have to pollute the test with it.

addressed in 9cd9bc2

however, I still need to keep this method, if I use the ClassLoader to add all the files loaded by default - the test will fail here

An aside: I think you could probably ship this method + runTasksViaGradlew into gradle-testing-plugin or a new thing. It'd just look very similar to IntegrationSpec or something.

…antir-java-format into cr/spotless-uses-java-if-21

felixdesouza · 2025-04-08T13:57:31Z

+switched to a native image for the formatter. The startup time for the native image esp. in Intellij is >10x faster than
+spinning up a new process that does the formatting.


I think this is a tad confusing, in both cases you're spinning up a new process.

Correct me if I've misunderstood, but I believe you want something along the lines of:

is >10x faster than spinning up a new JVM process that does the formatting

?

felixdesouza · 2025-04-08T13:58:21Z

+switched to a native image for the formatter. The startup time for the native image esp. in Intellij is >10x faster than
+spinning up a new process that does the formatting.
+However, the throughput of running the native image for a large set of files (eg. running `./gradlew spotlessApply`) is 
+considerably slower (eg. 30ms using the Java implementation vs 1m20s using the native image implementation). Therefore,


30 milliseconds vs 1 minute 20s?

felixdesouza · 2025-04-09T13:53:51Z

-        """.replace("EXTRA_CONFIGURATION", extraDependencies).stripIndent()
+
+            javaVersions {
+                libraryTarget = JAVA_VERSION


Can we just string interpolate here?

felixdesouza · 2025-04-24T14:30:53Z

+        File initScript = new File(projectDir, INIT_FILE_NAME)
+        ClasspathAddingInitScriptBuilder.build(initScript, getBuildPluginClasspathInjector().toList())


I think this should move into runGradlewTasks then you don't even need to have a constant. It's kinda similar to how the nebula thing does it, doesn't necessarily have to be a temp file, but I don't think it's anything specific to this particular test unlike CLASSPATH_FILE and NATIVE_CONFIG etc.

Invalidated by push of 113ee73

CRogers · 2025-05-15T16:30:25Z

👍

autorelease3 · 2025-05-15T16:30:42Z

Released 2.67.0

crogoz added 3 commits March 18, 2025 15:55

tests with java versions

d44a3fa

fix

e4a42d1

.

070c5cf

crogoz commented Mar 19, 2025

View reviewed changes

crogoz requested a review from CRogers March 19, 2025 09:00

crogoz changed the title ~~SpotlessApply uses Java 21 version~~ SpotlessApply uses by default the Java formatter Mar 19, 2025

svc-changelog and others added 3 commits March 19, 2025 09:01

Add generated changelog entries

17e9bdd

add readme explanation

6021843

Merge branch 'cr/spotless-uses-java-if-21' of github.com:palantir/pal…

66c2d7c

…antir-java-format into cr/spotless-uses-java-if-21

felixdesouza reviewed Apr 8, 2025

View reviewed changes

crogoz added 2 commits April 8, 2025 16:01

Update README.md

b9b5073

Update README.md

81ff9ae

felixdesouza reviewed Apr 9, 2025

View reviewed changes

crogoz added 2 commits April 11, 2025 15:53

use ClasspathAddingInitScriptBuilder

9cd9bc2

.

ea77c75

crogoz requested a review from felixdesouza April 22, 2025 12:08

felixdesouza reviewed Apr 24, 2025

View reviewed changes

crogoz added 2 commits April 25, 2025 11:49

fix

7d30bfb

cleanup

065d9c5

felixdesouza previously approved these changes Apr 25, 2025

View reviewed changes

fix

113ee73

crogoz added 2 commits May 15, 2025 16:21

fix

1dfcc88

fix waitFor

22132dc

crogoz added autorelease merge when ready labels May 15, 2025

bulldozer-bot Bot merged commit d2acb99 into develop May 15, 2025
9 checks passed

bulldozer-bot Bot deleted the cr/spotless-uses-java-if-21 branch May 15, 2025 16:30

gnodet referenced this pull request in apache/maven-parent Oct 15, 2025

Use spotless 3 when running on JDK >= 17 (#504)

e5188b3


		}

		private static Iterable<File> getBuildPluginClasspathInjector() {

		switched to a native image for the formatter. The startup time for the native image esp. in Intellij is >10x faster than
		spinning up a new process that does the formatting.

		File initScript = new File(projectDir, INIT_FILE_NAME)
		ClasspathAddingInitScriptBuilder.build(initScript, getBuildPluginClasspathInjector().toList())

Conversation

crogoz commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before this PR

After this PR

Possible downsides?

Uh oh!

changelog-app Bot commented Mar 19, 2025 • edited by crogoz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Generate changelog in changelog/@unreleased

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixdesouza Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CRogers commented May 15, 2025

Uh oh!

Uh oh!

autorelease3 Bot commented May 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

crogoz commented Mar 19, 2025 •

edited

Loading

changelog-app Bot commented Mar 19, 2025 •

edited by crogoz

Loading

Generate changelog in `changelog/@unreleased`

felixdesouza Apr 9, 2025 •

edited

Loading