From 4db7e8f94d3764e4ebdb51fb6f22b75d701bb248 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sat, 19 Apr 2014 18:24:43 -0700 Subject: [PATCH 1/5] blame: Only use computeRange if -L was requested The computeRange method is inefficient for computing the entire file. If the entire file was selected ask for the entire file. Change-Id: I8b2dbf635e875cc125443dac50be121208646540 --- org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameResult.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameResult.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameResult.java index 2d445fd7b..faaa227b4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameResult.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameResult.java @@ -290,6 +290,10 @@ public class BlameResult { BlameGenerator gen = generator; if (gen == null) return; + if (start == 0 && end == resultContents.size()) { + computeAll(); + return; + } while (start < end) { if (hasSourceData(start, end)) From 6afae79901f7d3fe7598c81f7474bc98c29909b9 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sat, 19 Apr 2014 19:59:21 -0700 Subject: [PATCH 2/5] blame: Do not update candidate regionList during output Instead of updating the candidate's regionList field to iterate through the linked list of regions, use a special purpose field in the BlameGenerator. This allows the candidate to be unmodified. Change-Id: I2cda031b59220ab603ef82050e741ecbbaa1953f --- .../eclipse/jgit/blame/BlameGenerator.java | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java index 5b02ce6c9..d90d59824 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java @@ -143,7 +143,8 @@ public class BlameGenerator { private int remaining; /** Blame is currently assigned to this source. */ - private Candidate currentSource; + private Candidate outCandidate; + private Region outRegion; /** * Create a blame generator for the repository and path (relative to @@ -466,19 +467,19 @@ public class BlameGenerator { */ public boolean next() throws IOException { // If there is a source still pending, produce the next region. - if (currentSource != null) { - Region r = currentSource.regionList; - Region n = r.next; + if (outRegion != null) { + Region r = outRegion; remaining -= r.length; - if (n != null) { - currentSource.regionList = n; + if (r.next != null) { + outRegion = r.next; return true; } - if (currentSource.queueNext != null) - return result(currentSource.queueNext); + if (outCandidate.queueNext != null) + return result(outCandidate.queueNext); - currentSource = null; + outCandidate = null; + outRegion = null; } // If there are no lines remaining, the entire result is done, @@ -520,7 +521,8 @@ public class BlameGenerator { private boolean result(Candidate n) throws IOException { if (n.sourceCommit != null) revPool.parseBody(n.sourceCommit); - currentSource = n; + outCandidate = n; + outRegion = n.regionList; return true; } @@ -847,12 +849,12 @@ public class BlameGenerator { * @return current revision being blamed. */ public RevCommit getSourceCommit() { - return currentSource.sourceCommit; + return outCandidate.sourceCommit; } /** @return current author being blamed. */ public PersonIdent getSourceAuthor() { - return currentSource.getAuthor(); + return outCandidate.getAuthor(); } /** @return current committer being blamed. */ @@ -863,12 +865,12 @@ public class BlameGenerator { /** @return path of the file being blamed. */ public String getSourcePath() { - return currentSource.sourcePath.getPath(); + return outCandidate.sourcePath.getPath(); } /** @return rename score if a rename occurred in {@link #getSourceCommit}. */ public int getRenameScore() { - return currentSource.renameScore; + return outCandidate.renameScore; } /** @@ -878,7 +880,7 @@ public class BlameGenerator { * {@link #getSourcePath()}. */ public int getSourceStart() { - return currentSource.regionList.sourceStart; + return outRegion.sourceStart; } /** @@ -888,7 +890,7 @@ public class BlameGenerator { * {@link #getSourcePath()}. */ public int getSourceEnd() { - Region r = currentSource.regionList; + Region r = outRegion; return r.sourceStart + r.length; } @@ -897,7 +899,7 @@ public class BlameGenerator { * blamed for providing. Line numbers use 0 based indexing. */ public int getResultStart() { - return currentSource.regionList.resultStart; + return outRegion.resultStart; } /** @@ -908,7 +910,7 @@ public class BlameGenerator { * than {@link #getResultStart()}. */ public int getResultEnd() { - Region r = currentSource.regionList; + Region r = outRegion; return r.resultStart + r.length; } @@ -919,7 +921,7 @@ public class BlameGenerator { * {@code getSourceEnd() - getSourceStart()}. */ public int getRegionLength() { - return currentSource.regionList.length; + return outRegion.length; } /** @@ -930,7 +932,7 @@ public class BlameGenerator { * applications will want the result contents for display to users. */ public RawText getSourceContents() { - return currentSource.sourceText; + return outCandidate.sourceText; } /** @@ -951,7 +953,8 @@ public class BlameGenerator { public void release() { revPool.release(); queue = null; - currentSource = null; + outCandidate = null; + outRegion = null; } private boolean find(RevCommit commit, PathFilter path) throws IOException { From 2ee4d4a2c732171cd33d373df49093ff8ff4b775 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sat, 19 Apr 2014 20:02:19 -0700 Subject: [PATCH 3/5] blame: Allow candidate to manage its setup before output Pass in the RevWalk and let the candidate decide how to prepare itself for output. This removes the conditional for the missing sourceCommit, as candidates missing a commit can override the method with a no-op. Change-Id: I3fa19b8676dfd3c177583f8f42593b5000b5350d --- .../src/org/eclipse/jgit/blame/BlameGenerator.java | 3 +-- .../src/org/eclipse/jgit/blame/Candidate.java | 11 +++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java index d90d59824..dd6717b6d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java @@ -519,8 +519,7 @@ public class BlameGenerator { } private boolean result(Candidate n) throws IOException { - if (n.sourceCommit != null) - revPool.parseBody(n.sourceCommit); + n.beginResult(revPool); outCandidate = n; outRegion = n.regionList; return true; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java index a652278d0..c7c34f0a3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java @@ -49,6 +49,7 @@ import org.eclipse.jgit.blame.ReverseWalk.ReverseCommit; import org.eclipse.jgit.diff.Edit; import org.eclipse.jgit.diff.EditList; import org.eclipse.jgit.diff.RawText; +import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; @@ -56,6 +57,7 @@ import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevFlag; +import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.filter.PathFilter; /** @@ -114,6 +116,10 @@ class Candidate { recursivePath = path.shouldBeRecursive(); } + void beginResult(RevWalk rw) throws MissingObjectException, IOException { + rw.parseBody(sourceCommit); + } + int getParentCount() { return sourceCommit.getParentCount(); } @@ -405,6 +411,11 @@ class Candidate { description = name; } + @Override + void beginResult(RevWalk rw) { + // Blob candidates have nothing to prepare. + } + @Override int getParentCount() { return parent != null ? 1 : 0; From 4d840fc6db6edf8fc924bdf8191d3c611029e63a Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sun, 20 Apr 2014 15:54:10 -0700 Subject: [PATCH 4/5] blame: Remove unnecessary curly braces around single statement if Change-Id: I8bcab44785fe08bbf3519c634e57ebfea8d3f0f9 --- .../src/org/eclipse/jgit/blame/BlameGenerator.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java index dd6717b6d..89a8846bc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java @@ -689,9 +689,8 @@ public class BlameGenerator { revPool.parseHeaders(parent); if (!find(parent, n.sourcePath)) continue; - if (!(n instanceof ReverseCandidate) && idBuf.equals(n.sourceBlob)) { + if (!(n instanceof ReverseCandidate) && idBuf.equals(n.sourceBlob)) return blameEntireRegionOnParent(n, parent); - } if (ids == null) ids = new ObjectId[pCnt]; ids[pIdx] = idBuf.toObjectId(); From 2d76133ba229135fcc8a9631fe0619456290f34d Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sun, 20 Apr 2014 15:55:32 -0700 Subject: [PATCH 5/5] blame: Reuse existing blameEntireRegionOnParent method Skipping directly to the parent is already possible with an existing helper method. Update the source path (to follow the rename) and then use the existing code path to push the parent inside the current entry. Change-Id: Icb1d49e53d14b599efc478990613625a9e058e09 --- .../src/org/eclipse/jgit/blame/BlameGenerator.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java index 89a8846bc..ca695d2a8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java @@ -720,10 +720,8 @@ public class BlameGenerator { // have an exact content match. For performance reasons // we choose to follow the one parent over trying to do // possibly both parents. - n.sourceCommit = parent; n.setSourcePath(PathFilter.create(r.getOldPath())); - push(n); - return false; + return blameEntireRegionOnParent(n, parent); } renames[pIdx] = r;