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

Improve the git ratchet performance (memory and speed) #1038

Merged
merged 5 commits into from Dec 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
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
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
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
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
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
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
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
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