Cache blame results The cache is weighted based on the number of distinct regions. Change-Id: Ie9a26c0667576646792ba3639075d09fde153cae
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/BlameCache.java b/gitiles-servlet/src/main/java/com/google/gitiles/BlameCache.java new file mode 100644 index 0000000..f330839 --- /dev/null +++ b/gitiles-servlet/src/main/java/com/google/gitiles/BlameCache.java
@@ -0,0 +1,169 @@ +// Copyright (C) 2014 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gitiles; + +import com.google.common.base.Objects; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; +import com.google.common.cache.Weigher; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; + +import org.eclipse.jgit.blame.BlameGenerator; +import org.eclipse.jgit.blame.BlameResult; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.ExecutionException; + +/** Cache of blame data, weighted by number of blame regions. */ +public class BlameCache { + public static CacheBuilder<Object, Object> newBuilder() { + return CacheBuilder.newBuilder().maximumWeight(10 << 10); + } + + static class Region { + private final String sourcePath; + private final ObjectId sourceCommit; + private final PersonIdent sourceAuthor; + private int count; + + Region(BlameResult blame, int start) { + this.sourcePath = blame.getSourcePath(start); + RevCommit c = blame.getSourceCommit(start); + this.sourceCommit = c.copy(); + this.sourceAuthor = c.getAuthorIdent(); + this.count = 1; + } + + int getCount() { + return count; + } + + String getSourcePath() { + return sourcePath; + } + + ObjectId getSourceCommit() { + return sourceCommit; + } + + PersonIdent getSourceAuthor() { + return sourceAuthor; + } + + private boolean growFrom(BlameResult blame, int i) { + // Don't compare line numbers, so we collapse regions from the same source + // but with deleted lines into one. + if (Objects.equal(blame.getSourcePath(i), sourcePath) + && Objects.equal(blame.getSourceCommit(i), sourceCommit)) { + count++; + return true; + } else { + return false; + } + } + } + + private static class Key { + private final ObjectId commitId; + private final String path; + private Repository repo; + + private Key(Repository repo, ObjectId commitId, String path) { + this.commitId = commitId; + this.path = path; + this.repo = repo; + } + + @Override + public boolean equals(Object o) { + if (o instanceof Key) { + Key k = (Key) o; + return Objects.equal(commitId, k.commitId) + && Objects.equal(path, k.path); + } + return false; + } + + @Override + public int hashCode() { + return Objects.hashCode(commitId, path); + } + } + + private final LoadingCache<Key, List<BlameCache.Region>> cache; + + public BlameCache() { + this(newBuilder()); + } + + public BlameCache(CacheBuilder<Object, Object> builder) { + this.cache = builder.weigher(new Weigher<Key, List<BlameCache.Region>>() { + @Override + public int weigh(Key key, List<BlameCache.Region> value) { + return value.size(); + } + }).build(new CacheLoader<Key, List<BlameCache.Region>>() { + @Override + public List<BlameCache.Region> load(Key key) throws IOException { + return loadBlame(key); + } + }); + } + + List<BlameCache.Region> get(Repository repo, ObjectId commitId, String path) + throws IOException { + try { + return cache.get(new Key(repo, commitId, path)); + } catch (ExecutionException e) { + throw new IOException(e); + } + } + + private List<BlameCache.Region> loadBlame(Key key) throws IOException { + try { + BlameGenerator gen = new BlameGenerator(key.repo, key.path); + BlameResult blame; + try { + gen.push(null, key.commitId); + blame = gen.computeBlameResult(); + } finally { + gen.release(); + } + if (blame == null) { + return ImmutableList.of(); + } + int lineCount = blame.getResultContents().size(); + blame.discardResultContents(); + + List<BlameCache.Region> regions = Lists.newArrayList(); + for (int i = 0; i < lineCount; i++) { + if (regions.isEmpty() || !regions.get(regions.size() - 1).growFrom(blame, i)) { + regions.add(new BlameCache.Region(blame, i)); + } + } + return Collections.unmodifiableList(regions); + } finally { + key.repo = null; + } + } +}
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/BlameServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/BlameServlet.java index c73f7c0..6892b25 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/BlameServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/BlameServlet.java
@@ -14,15 +14,14 @@ package com.google.gitiles; +import static com.google.common.base.Preconditions.checkNotNull; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; -import com.google.common.base.Objects; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.gitiles.BlameCache.Region; -import org.eclipse.jgit.blame.BlameGenerator; -import org.eclipse.jgit.blame.BlameResult; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.http.server.ServletUtils; import org.eclipse.jgit.lib.Config; @@ -47,8 +46,11 @@ public class BlameServlet extends BaseServlet { private static final long serialVersionUID = 1L; - public BlameServlet(Config cfg, Renderer renderer) { + private final BlameCache cache; + + public BlameServlet(Config cfg, Renderer renderer, BlameCache cache) { super(cfg, renderer); + this.cache = checkNotNull(cache, "cache"); } @Override @@ -59,7 +61,8 @@ RevWalk rw = new RevWalk(repo); try { - ObjectId blobId = resolveBlob(view, rw); + RevCommit commit = rw.parseCommit(view.getRevision().getId()); + ObjectId blobId = resolveBlob(view, rw, commit); if (blobId == null) { res.setStatus(SC_NOT_FOUND); return; @@ -68,19 +71,17 @@ String title = "Blame - " + view.getPathPart(); Map<String, ?> blobData = new BlobSoyData(rw, view).toSoyData(view.getPathPart(), blobId); if (blobData.get("data") != null) { - BlameResult blame = doBlame(repo, view); - if (blame == null) { + List<Region> regions = cache.get(repo, commit, view.getPathPart()); + if (regions.isEmpty()) { res.setStatus(SC_NOT_FOUND); return; } GitDateFormatter df = new GitDateFormatter(Format.DEFAULT); - int lineCount = blame.getResultContents().size(); - blame.discardResultContents(); renderHtml(req, res, "gitiles.blameDetail", ImmutableMap.of( "title", title, "breadcrumbs", view.getBreadcrumbs(), "data", blobData, - "regions", toRegionData(view, rw.getObjectReader(), blame, lineCount, df))); + "regions", toSoyData(view, rw.getObjectReader(), regions, df))); } else { renderHtml(req, res, "gitiles.blameDetail", ImmutableMap.of( "title", title, @@ -92,10 +93,10 @@ } } - private static ObjectId resolveBlob(GitilesView view, RevWalk rw) throws IOException { + private static ObjectId resolveBlob(GitilesView view, RevWalk rw, RevCommit commit) + throws IOException { try { - TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), view.getPathPart(), - rw.parseTree(view.getRevision().getId())); + TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), view.getPathPart(), commit.getTree()); if ((tw.getRawMode(0) & FileMode.TYPE_FILE) == 0) { return null; } @@ -105,79 +106,31 @@ } } - private static BlameResult doBlame(Repository repo, GitilesView view) throws IOException { - BlameGenerator gen = new BlameGenerator(repo, view.getPathPart()); - BlameResult blame; - try { - // TODO: works on annotated tag? - gen.push(null, view.getRevision().getId()); - blame = gen.computeBlameResult(); - } finally { - gen.release(); - } - return blame; - } - - private List<Map<String, ?>> toRegionData(GitilesView view, ObjectReader reader, - BlameResult blame, int lineCount, GitDateFormatter df) throws IOException { - List<Region> regions = Lists.newArrayList(); - for (int i = 0; i < lineCount; i++) { - if (regions.isEmpty() || !regions.get(regions.size() - 1).growFrom(blame, i)) { - regions.add(new Region(blame, i)); - } - } - + private static List<Map<String, ?>> toSoyData(GitilesView view, ObjectReader reader, + List<Region> regions, GitDateFormatter df) throws IOException { Map<ObjectId, String> abbrevShas = Maps.newHashMap(); List<Map<String, ?>> result = Lists.newArrayListWithCapacity(regions.size()); - for (Region r : regions) { - result.add(r.toSoyData(view, reader, abbrevShas, df)); - } - return result; - } - - private class Region { - private final String sourcePath; - private final RevCommit sourceCommit; - private int count; - - private Region(BlameResult blame, int start) { - this.sourcePath = blame.getSourcePath(start); - this.sourceCommit = blame.getSourceCommit(start); - this.count = 1; - } - - private boolean growFrom(BlameResult blame, int i) { - // Don't compare line numbers, so we collapse regions from the same source - // but with deleted lines into one. - if (Objects.equal(blame.getSourcePath(i), sourcePath) - && Objects.equal(blame.getSourceCommit(i), sourceCommit)) { - count++; - return true; - } else { - return false; - } - } - - private Map<String, ?> toSoyData(GitilesView view, ObjectReader reader, - Map<ObjectId, String> abbrevShas, GitDateFormatter df) throws IOException { - if (sourceCommit == null) { + for (BlameCache.Region r : regions) { + if (r.getSourceCommit() == null) { // JGit bug may fail to blame some regions. We should fix this // upstream, but handle it for now. - return ImmutableMap.of("count", count); + result.add(ImmutableMap.of("count", r.getCount())); + } else { + String abbrevSha = abbrevShas.get(r.getSourceCommit()); + if (abbrevSha == null) { + abbrevSha = reader.abbreviate(r.getSourceCommit()).name(); + abbrevShas.put(r.getSourceCommit(), abbrevSha); + } + result.add(ImmutableMap.of( + "abbrevSha", abbrevSha, + "url", GitilesView.blame().copyFrom(view) + .setRevision(r.getSourceCommit().name()) + .setPathPart(r.getSourcePath()) + .toUrl(), + "author", CommitSoyData.toSoyData(r.getSourceAuthor(), df), + "count", r.getCount())); } - String abbrevSha = abbrevShas.get(sourceCommit); - if (abbrevSha == null) { - abbrevSha = reader.abbreviate(sourceCommit).name(); - abbrevShas.put(sourceCommit, abbrevSha); - } - return ImmutableMap.of( - "abbrevSha", abbrevSha, - "url", GitilesView.blame().copyFrom(view) - .setRevision(sourceCommit.name()) - .setPathPart(sourcePath) - .toUrl(), - "author", CommitSoyData.toSoyData(sourceCommit.getAuthorIdent(), df), - "count", count); } + return result; } }
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java index 20212b6..ca17e33 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java
@@ -162,6 +162,7 @@ private RepositoryResolver<HttpServletRequest> resolver; private VisibilityCache visibilityCache; private TimeCache timeCache; + private BlameCache blameCache; private GitwebRedirectFilter gitwebRedirect; private boolean initialized; @@ -245,7 +246,7 @@ case ARCHIVE: return new ArchiveServlet(config); case BLAME: - return new BlameServlet(config, renderer); + return new BlameServlet(config, renderer, blameCache); default: throw new IllegalArgumentException("Invalid view type: " + view); } @@ -299,6 +300,7 @@ setDefaultAccess(); setDefaultVisbilityCache(); setDefaultTimeCache(); + setDefaultBlameCache(); setDefaultGitwebRedirect(); } @@ -388,6 +390,16 @@ } } + private void setDefaultBlameCache() { + if (blameCache == null) { + if (config.getSubsections("cache").contains("blame")) { + blameCache = new BlameCache(ConfigUtil.getCacheBuilder(config, "blame")); + } else { + blameCache = new BlameCache(); + } + } + } + private void setDefaultGitwebRedirect() { if (gitwebRedirect == null) { if (config.getBoolean("gitiles", null, "redirectGitweb", true)) {