Skip to content

Commit

Permalink
Improve the git ratchet performance (memory and speed) (#1038)
Browse files Browse the repository at this point in the history
  • Loading branch information
nedtwigg authored Dec 16, 2021
2 parents 3f772a7 + 80afe4b commit 00c2d3f
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 20 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ public boolean isClean(Project project, ObjectId treeSha, File file) throws IOEx
return isClean(project, treeSha, relativePath);
}

private Map<Repository, DirCache> 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
Expand All @@ -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),
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
/**
Expand All @@ -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);
Expand All @@ -107,7 +103,7 @@ public void setupRatchet(String ratchetFrom) {

@Internal
GitRatchetGradle getRatchet() {
return ratchet;
return ObjectId.zeroId().equals(getRatchetSha()) ? null : getTaskService().get().getRatchet();
}

@Internal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -53,7 +55,8 @@ void init(Provider<SpotlessTaskService> 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<File> target'");
}
Expand All @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions plugin-maven/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 00c2d3f

Please sign in to comment.