diff --git a/.circleci/config.yml b/.circleci/config.yml index 4667ee2fc5..2fe588a448 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -109,7 +109,7 @@ jobs: - *restore_cache_deps - run: name: gradlew npmTest - command: ./gradlew npmTest --build-cache + command: export SPOTLESS_EXCLUDE_MAVEN=true && ./gradlew npmTest --build-cache - store_test_results: path: testlib/build/test-results/NpmTest - store_test_results: diff --git a/CHANGES.md b/CHANGES.md index a187f62b23..25cf0ccdea 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,8 @@ This document is intended for Spotless developers. We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +### Fixed +* Performance improvements to GitRatchet ([#1038](https://github.com/diffplug/spotless/pull/1038)). ## [2.20.2] - 2021-12-05 ### Changed diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/GitRatchet.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/GitRatchet.java index 5ce8173845..6b8d5a9f41 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/GitRatchet.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/GitRatchet.java @@ -63,6 +63,8 @@ public boolean isClean(Project project, ObjectId treeSha, File file) throws IOEx return isClean(project, treeSha, relativePath); } + private Map dirCaches = new HashMap<>(); + /** * This is the highest-level method, which all the others serve. Given the sha * of a git tree (not a commit!), and the file in question, this method returns @@ -72,13 +74,16 @@ public boolean isClean(Project project, ObjectId treeSha, File file) throws IOEx public boolean isClean(Project project, ObjectId treeSha, String relativePathUnix) throws IOException { Repository repo = repositoryFor(project); - // TODO: should be cached-per-repo if it is thread-safe, or per-repo-per-thread if it is not - DirCache dirCache = repo.readDirCache(); - + DirCacheIterator dirCacheIteratorInit; + synchronized (this) { + // each DirCache is thread-safe, and we compute them one-to-one based on `repositoryFor` + DirCache dirCache = dirCaches.computeIfAbsent(repo, Errors.rethrow().wrap(Repository::readDirCache)); + dirCacheIteratorInit = new DirCacheIterator(dirCache); + } try (TreeWalk treeWalk = new TreeWalk(repo)) { treeWalk.setRecursive(true); treeWalk.addTree(treeSha); - treeWalk.addTree(new DirCacheIterator(dirCache)); + treeWalk.addTree(dirCacheIteratorInit); treeWalk.addTree(new FileTreeIterator(repo)); treeWalk.setFilter(AndTreeFilter.create( PathFilter.create(relativePathUnix), @@ -216,7 +221,7 @@ public synchronized ObjectId rootTreeShaOf(Project project, String reference) { RevCommit mergeBase = revWalk.next(); treeSha = Optional.ofNullable(mergeBase).orElse(ratchetFrom).getTree(); } - rootTreeShaCache.put(repo, reference, treeSha); + rootTreeShaCache.put(repo, reference, treeSha.copy()); } return treeSha; } catch (IOException e) { @@ -241,7 +246,7 @@ public synchronized ObjectId subtreeShaOf(Project project, ObjectId rootTreeSha) TreeWalk treeWalk = TreeWalk.forPath(repo, subpath, rootTreeSha); subtreeSha = treeWalk == null ? ObjectId.zeroId() : treeWalk.getObjectId(0); } - subtreeShaCache.put(project, subtreeSha); + subtreeShaCache.put(project, subtreeSha.copy()); } return subtreeSha; } catch (IOException e) { diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 5b1a0a8adf..a1f7a7e730 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -3,6 +3,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`). ## [Unreleased] +### Fixed +* `ratchetFrom` is now faster and uses less memory ([#1038](https://github.com/diffplug/spotless/pull/1038)). ## [6.0.4] - 2021-12-07 ### Fixed diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java index dd62cd523a..8d24e2230e 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java @@ -41,9 +41,10 @@ static void performHook(SpotlessTaskImpl spotlessTask) { return; } if (spotlessTask.getTarget().contains(file)) { + GitRatchetGradle ratchet = spotlessTask.getRatchet(); try (Formatter formatter = spotlessTask.buildFormatter()) { - if (spotlessTask.getRatchet() != null) { - if (spotlessTask.getRatchet().isClean(spotlessTask.getProjectDir().get().getAsFile(), spotlessTask.getRootTreeSha(), file)) { + if (ratchet != null) { + if (ratchet.isClean(spotlessTask.getProjectDir().get().getAsFile(), spotlessTask.getRootTreeSha(), file)) { dumpIsClean(); return; } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index fe5628bb83..6f2279b2e0 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -23,8 +23,6 @@ import java.util.Locale; import java.util.Objects; -import javax.annotation.Nullable; - import org.eclipse.jgit.lib.ObjectId; import org.gradle.api.DefaultTask; import org.gradle.api.file.DirectoryProperty; @@ -44,6 +42,7 @@ import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; +import com.diffplug.spotless.extra.GitRatchet; public abstract class SpotlessTask extends DefaultTask { @Internal @@ -76,9 +75,6 @@ public void setLineEndingsPolicy(LineEnding.Policy lineEndingsPolicy) { this.lineEndingsPolicy.set(lineEndingsPolicy); } - /*** API which performs git up-to-date tasks. */ - @Nullable - private transient GitRatchetGradle ratchet; /** The sha of the tree at repository root, used for determining if an individual *file* is clean according to git. */ private transient ObjectId rootTreeSha; /** @@ -93,7 +89,7 @@ public void setLineEndingsPolicy(LineEnding.Policy lineEndingsPolicy) { public void setupRatchet(String ratchetFrom) { this.ratchetFrom = ratchetFrom; if (!ratchetFrom.isEmpty()) { - ratchet = getTaskService().get().getRatchet(); + GitRatchet ratchet = getTaskService().get().getRatchet(); File projectDir = getProjectDir().get().getAsFile(); rootTreeSha = ratchet.rootTreeShaOf(projectDir, ratchetFrom); subtreeSha = ratchet.subtreeShaOf(projectDir, rootTreeSha); @@ -107,7 +103,7 @@ public void setupRatchet(String ratchetFrom) { @Internal GitRatchetGradle getRatchet() { - return ratchet; + return ObjectId.zeroId().equals(getRatchetSha()) ? null : getTaskService().get().getRatchet(); } @Internal diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index a9bf428caa..1b9cda204c 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -21,6 +21,7 @@ import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import javax.annotation.Nullable; import javax.inject.Inject; import org.gradle.api.GradleException; @@ -37,6 +38,7 @@ import com.diffplug.common.base.StringPrinter; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.PaddedCell; +import com.diffplug.spotless.extra.GitRatchet; @CacheableTask public abstract class SpotlessTaskImpl extends SpotlessTask { @@ -53,7 +55,8 @@ void init(Provider service) { @TaskAction public void performAction(InputChanges inputs) throws Exception { - getTaskService().get().registerSourceAlreadyRan(this); + SpotlessTaskService taskService = getTaskService().get(); + taskService.registerSourceAlreadyRan(this); if (target == null) { throw new GradleException("You must specify 'Iterable target'"); } @@ -65,24 +68,25 @@ public void performAction(InputChanges inputs) throws Exception { } try (Formatter formatter = buildFormatter()) { + GitRatchetGradle ratchet = getRatchet(); for (FileChange fileChange : inputs.getFileChanges(target)) { File input = fileChange.getFile(); if (fileChange.getChangeType() == ChangeType.REMOVED) { deletePreviousResult(input); } else { if (input.isFile()) { - processInputFile(formatter, input); + processInputFile(ratchet, formatter, input); } } } } } - private void processInputFile(Formatter formatter, File input) throws IOException { + private void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File input) throws IOException { File output = getOutputFile(input); getLogger().debug("Applying format to " + input + " and writing to " + output); PaddedCell.DirtyState dirtyState; - if (getRatchet() != null && getRatchet().isClean(getProjectDir().get().getAsFile(), getRootTreeSha(), input)) { + if (ratchet != null && ratchet.isClean(getProjectDir().get().getAsFile(), getRootTreeSha(), input)) { dirtyState = PaddedCell.isClean(); } else { dirtyState = PaddedCell.calculateDirtyState(formatter, input); diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index bf76b48938..1da76e4fd6 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -3,6 +3,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +### Fixed +* `ratchetFrom` is now faster ([#1038](https://github.com/diffplug/spotless/pull/1038)). ## [2.17.6] - 2021-12-05 ### Changed