Skip to content

Commit

Permalink
Merge pull request #597 from diffplug/feat/git-speedup
Browse files Browse the repository at this point in the history
Speed up `ratchetFrom` using subtree as key
  • Loading branch information
nedtwigg authored Jun 4, 2020
2 parents fcbf0dc + 10f229b commit 8515a0f
Show file tree
Hide file tree
Showing 7 changed files with 270 additions and 79 deletions.
3 changes: 2 additions & 1 deletion plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (

## [Unreleased]
### Fixed
* `ratchetFrom` incorrectly marked every file as though it were clean on Windows.
* `ratchetFrom` incorrectly marked every file as though it were clean on Windows. ([#596](https://github.com/diffplug/spotless/pull/596))
* Also large [performance improvement (win and unix) for multiproject builds](https://github.com/diffplug/spotless/pull/597/commits/f66dc8de137a34d14768e83ab3cbff5344539b56). ([#597](https://github.com/diffplug/spotless/pull/597))
* Improved the warning message for `paddedCell` deprecation, along with many API-invisible fixes and cleanup. ([#592](https://github.com/diffplug/spotless/pull/592))

## [4.2.0] - 2020-06-03
Expand Down
3 changes: 3 additions & 0 deletions plugin-gradle/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ task npmTest(type: Test) {
}
}

// make it easy for eclipse to run against latest build
tasks.eclipse.dependsOn(pluginUnderTestMetadata)

//////////////////////////
// GRADLE PLUGIN PORTAL //
//////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,7 @@ protected void setupTask(SpotlessTask task) {
spotless.registerDependenciesTask.hookSubprojectTask(task);
}
if (spotless.getRatchetFrom() != null) {
task.ratchet = spotless.registerDependenciesTask.gitRatchet;
task.treeSha = task.ratchet.treeShaOf(spotless.project, spotless.getRatchetFrom());
task.setupRatchet(spotless.registerDependenciesTask.gitRatchet, spotless.getRatchetFrom());
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2020 DiffPlug
* Copyright 2020 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,8 +17,9 @@

import java.io.File;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.TreeMap;

import javax.annotation.Nullable;

Expand Down Expand Up @@ -117,8 +118,9 @@ private static boolean worktreeIsCleanCheckout(TreeWalk treeWalk) {
private final static int INDEX = 1;
private final static int WORKDIR = 2;

TreeMap<Project, Repository> gitRoots = new TreeMap<>();
Table<Repository, String, ObjectId> shaCache = HashBasedTable.create();
Map<Project, Repository> gitRoots = new HashMap<>();
Table<Repository, String, ObjectId> rootTreeShaCache = HashBasedTable.create();
Map<Project, ObjectId> subtreeShaCache = new HashMap<>();

/**
* The first part of making this fast is finding the appropriate git repository quickly. Because of composite
Expand Down Expand Up @@ -175,24 +177,49 @@ static Repository createRepo(File dir) throws IOException {
* is the only method which can trigger any changes, and it is only called during project evaluation. That means our state
* is final/read-only during task execution, so we don't need any locks during the heavy lifting.
*/
public synchronized ObjectId treeShaOf(Project project, String reference) {
public synchronized ObjectId rootTreeShaOf(Project project, String reference) {
try {
Repository repo = repositoryFor(project);
ObjectId treeSha = shaCache.get(repo, reference);
ObjectId treeSha = rootTreeShaCache.get(repo, reference);
if (treeSha == null) {
ObjectId commitSha = repo.resolve(reference);
try (RevWalk revWalk = new RevWalk(repo)) {
RevCommit revCommit = revWalk.parseCommit(commitSha);
treeSha = revCommit.getTree();
}
shaCache.put(repo, reference, treeSha);
rootTreeShaCache.put(repo, reference, treeSha);
}
return treeSha;
} catch (Exception e) {
throw Errors.asRuntime(e);
}
}

/**
* Returns the sha of the git subtree which represents the root of the given project, or {@link ObjectId#zeroId()}
* if there is no git subtree at the project root.
*/
public synchronized ObjectId subtreeShaOf(Project project, ObjectId rootTreeSha) {
try {
ObjectId subtreeSha = subtreeShaCache.get(project);
if (subtreeSha == null) {
Repository repo = repositoryFor(project);
File directory = project.getProjectDir();
if (repo.getWorkTree().equals(directory)) {
subtreeSha = rootTreeSha;
} else {
String subpath = FileSignature.pathNativeToUnix(repo.getWorkTree().toPath().relativize(directory.toPath()).toString());
TreeWalk treeWalk = TreeWalk.forPath(repo, subpath, rootTreeSha);
subtreeSha = treeWalk == null ? ObjectId.zeroId() : treeWalk.getObjectId(0);
}
subtreeShaCache.put(project, subtreeSha);
}
return subtreeSha;
} catch (Exception e) {
throw Errors.asRuntime(e);
}
}

@Override
public void close() {
gitRoots.values().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ static void performHook(SpotlessTask spotlessTask) {
if (spotlessTask.getTarget().contains(file)) {
try (Formatter formatter = spotlessTask.buildFormatter()) {
if (spotlessTask.ratchet != null) {
if (spotlessTask.ratchet.isClean(spotlessTask.getProject(), spotlessTask.treeSha, file)) {
if (spotlessTask.ratchet.isClean(spotlessTask.getProject(), spotlessTask.rootTreeSha, file)) {
dumpIsClean();
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,18 +91,37 @@ public void setLineEndingsPolicy(LineEnding.Policy lineEndingsPolicy) {
this.lineEndingsPolicy = Objects.requireNonNull(lineEndingsPolicy);
}

/*** API which performs git up-to-date tasks. */
@Nullable
GitRatchet ratchet;
ObjectId treeSha = ObjectId.zeroId();
/** The sha of the tree at repository root, used for determining if an individual *file* is clean according to git. */
ObjectId rootTreeSha;
/**
* The sha of the tree at the root of *this project*, used to determine if the git baseline has changed within this folder.
* Using a more fine-grained tree (rather than the project root) allows Gradle to mark more subprojects as up-to-date
* compared to using the project root.
*/
private ObjectId subtreeSha = ObjectId.zeroId();

public void setupRatchet(GitRatchet gitRatchet, String ratchetFrom) {
ratchet = gitRatchet;
rootTreeSha = gitRatchet.rootTreeShaOf(getProject(), ratchetFrom);
subtreeSha = gitRatchet.subtreeShaOf(getProject(), rootTreeSha);
}

@Internal
public GitRatchet getRatchet() {
GitRatchet getRatchet() {
return ratchet;
}

@Internal
ObjectId getRootTreeSha() {
return rootTreeSha;
}

@Input
public ObjectId getRatchetSha() {
return treeSha;
return subtreeSha;
}

@Deprecated
Expand Down Expand Up @@ -202,7 +221,7 @@ public void performAction(IncrementalTaskInputs inputs) throws Exception {
if (this.filePatterns.isEmpty()) {
shouldInclude = file -> true;
} else {
Preconditions.checkArgument(treeSha == ObjectId.zeroId(),
Preconditions.checkArgument(ratchet == null,
"Cannot use 'ratchetFrom' and '-PspotlessFiles' at the same time");

// a list of files has been passed in via project property
Expand Down Expand Up @@ -245,7 +264,7 @@ private void processInputFile(Formatter formatter, File input) throws IOExceptio
File output = getOutputFile(input);
getLogger().debug("Applying format to " + input + " and writing to " + output);
PaddedCell.DirtyState dirtyState;
if (ratchet != null && ratchet.isClean(getProject(), treeSha, input)) {
if (ratchet != null && ratchet.isClean(getProject(), rootTreeSha, input)) {
dirtyState = PaddedCell.isClean();
} else {
dirtyState = PaddedCell.calculateDirtyState(formatter, input);
Expand Down
Loading

0 comments on commit 8515a0f

Please sign in to comment.