Display logs in a streaming fashion JGit even on a fast workstation can only walk a few thousand commits per second with rename detection on (and in a loaded server environment it might be much slower). Loading a full page of 100 log results for a file therefore might take many seconds. Stream the output one log entry at a time so the page becomes interactive slightly faster. Each HTTP chunk is a full <li></li> tag, so browsers should be able to render incrementally. This is much simpler than an alternative solution involving AJAX to make multiple requests to the server, particularly in a multi-server cluster environment where the client is not guaranteed to talk to the same server (with the necessary RevWalk state in memory) on consecutive requests. Change-Id: I63c4bc655efd00453b6db60f333ba3dd5041e70a
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/LogServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/LogServlet.java index 519dc6d..c72a118 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/LogServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/LogServlet.java
@@ -49,6 +49,9 @@ import org.slf4j.LoggerFactory; import java.io.IOException; +import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.Writer; import java.util.Collection; import java.util.List; import java.util.Map; @@ -90,8 +93,7 @@ // Allow the user to select a logView variant with the "pretty" param. String pretty = Iterables.getFirst(view.getParameters().get(PRETTY_PARAM), "default"); - Map<String, Object> data = new LogSoyData(req, access, pretty) - .toSoyData(paginator, null, df); + Map<String, Object> data = Maps.newHashMapWithExpectedSize(2); if (!view.getRevision().nameIsId()) { List<Map<String, Object>> tags = Lists.newArrayListWithExpectedSize(1); @@ -114,6 +116,12 @@ data.put("title", title); + try (OutputStream out = startRenderStreamingHtml(req, res, "gitiles.logDetail", data); + Writer w = new OutputStreamWriter(out)) { + new LogSoyData(req, access, pretty) + .renderStreaming(paginator, null, renderer, w, df); + } + renderHtml(req, res, "gitiles.logDetail", data); } catch (RevWalkException e) { log.warn("Error in rev walk", e);
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/LogSoyData.java b/gitiles-servlet/src/main/java/com/google/gitiles/LogSoyData.java index a2ef5cf..6bb64b4 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/LogSoyData.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/LogSoyData.java
@@ -17,19 +17,19 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.base.Objects; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.gitiles.CommitData.Field; +import com.google.template.soy.tofu.SoyTofu; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevWalk; import java.io.IOException; -import java.util.List; +import java.io.Writer; import java.util.Map; import java.util.Set; @@ -43,6 +43,7 @@ private static final ImmutableSet<Field> VERBOSE_FIELDS = Field.setOf(FIELDS, Field.DIFF_TREE); private final HttpServletRequest req; + private final GitilesView view; private final Set<Field> fields; private final String pretty; private final String variant; @@ -50,44 +51,42 @@ public LogSoyData(HttpServletRequest req, GitilesAccess access, String pretty) throws IOException { this.req = checkNotNull(req); + this.view = checkNotNull(ViewFilter.getView(req)); this.pretty = checkNotNull(pretty); Config config = access.getConfig(); fields = config.getBoolean("logFormat", pretty, "verbose", false) ? VERBOSE_FIELDS : FIELDS; variant = Objects.firstNonNull(config.getString("logFormat", pretty, "variant"), pretty); } - public Map<String, Object> toSoyData(RevWalk walk, int limit, @Nullable String revision, - @Nullable ObjectId start, DateFormatter df) throws IOException { - return toSoyData(new Paginator(walk, limit, start), revision, df); + public void renderStreaming(Paginator paginator, @Nullable String revision, Renderer renderer, + Writer out, DateFormatter df) throws IOException { + renderer.newRenderer("gitiles.logEntriesHeader") + .setData(toHeaderSoyData(paginator, revision)) + .render(out); + out.flush(); + + SoyTofu.Renderer entryRenderer = renderer.newRenderer("gitiles.logEntryWrapper"); + boolean first = true; + for (RevCommit c : paginator) { + entryRenderer.setData(toEntrySoyData(paginator, c, df, first)).render(out); + out.flush(); + first = false; + } + if (first) { + renderer.newRenderer("gitiles.emptyLog").render(out); + } + + renderer.newRenderer("gitiles.logEntriesFooter") + .setData(toFooterSoyData(paginator, revision)) + .render(out); } - public Map<String, Object> toSoyData(Paginator paginator, @Nullable String revision, - DateFormatter df) throws IOException { - Map<String, Object> data = Maps.newHashMapWithExpectedSize(3); + private Map<String, Object> toHeaderSoyData(Paginator paginator, @Nullable String revision) { + Map<String, Object> data = Maps.newHashMapWithExpectedSize(5); data.put("logEntryPretty", pretty); - data.put("logEntryVariant", variant); - - List<Map<String, Object>> entries = Lists.newArrayListWithCapacity(paginator.getLimit()); - for (RevCommit c : paginator) { - Map<String, Object> entry = new CommitSoyData().setRevWalk(paginator.getWalk()) - .toSoyData(req, c, fields, df); - if (!entry.containsKey("diffTree")) { - entry.put("diffTree", null); - } - entries.add(entry); - } - data.put("entries", entries); - - GitilesView view = ViewFilter.getView(req); - ObjectId next = paginator.getNextStart(); - if (next != null) { - data.put("nextUrl", copyAndCanonicalize(view, revision) - .replaceParam(LogServlet.START_PARAM, next.name()) - .toUrl()); - } ObjectId prev = paginator.getPreviousStart(); if (prev != null) { - GitilesView.Builder prevView = copyAndCanonicalize(view, revision); + GitilesView.Builder prevView = copyAndCanonicalizeView(revision); if (!prevView.getRevision().getId().equals(prev)) { prevView.replaceParam(LogServlet.START_PARAM, prev.name()); } @@ -96,7 +95,28 @@ return data; } - private static GitilesView.Builder copyAndCanonicalize(GitilesView view, String revision) { + private Map<String, Object> toEntrySoyData(Paginator paginator, RevCommit c, DateFormatter df, + boolean first) throws IOException { + Map<String, Object> entry = new CommitSoyData().setRevWalk(paginator.getWalk()) + .toSoyData(req, c, fields, df); + return ImmutableMap.of( + "firstWithPrevious", first && paginator.getPreviousStart() != null, + "variant", variant, + "entry", entry); + } + + private Map<String, Object> toFooterSoyData(Paginator paginator, @Nullable String revision) { + Map<String, Object> data = Maps.newHashMapWithExpectedSize(1); + ObjectId next = paginator.getNextStart(); + if (next != null) { + data.put("nextUrl", copyAndCanonicalizeView(revision) + .replaceParam(LogServlet.START_PARAM, next.name()) + .toUrl()); + } + return data; + } + + private GitilesView.Builder copyAndCanonicalizeView(String revision) { // Canonicalize the view by using full SHAs. GitilesView.Builder copy = GitilesView.log().copyFrom(view); if (view.getRevision() != Revision.NULL) {
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java index ec565b3..0a4003d 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java
@@ -16,7 +16,6 @@ import static com.google.common.base.Preconditions.checkNotNull; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; @@ -32,6 +31,9 @@ import org.eclipse.jgit.revwalk.RevWalk; import java.io.IOException; +import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.Writer; import java.util.List; import java.util.Map; @@ -55,7 +57,58 @@ @Override protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException { - renderHtml(req, res, "gitiles.repositoryIndex", buildData(req)); + GitilesView view = ViewFilter.getView(req); + Repository repo = ServletUtils.getRepository(req); + GitilesAccess access = getAccess(req); + RepositoryDescription desc = access.getRepositoryDescription(); + + RevWalk walk = new RevWalk(repo); + Paginator paginator = null; + try { + Map<String, Object> data = Maps.newHashMapWithExpectedSize(7); + List<Map<String, Object>> tags = RefServlet.getTagsSoyData(req, timeCache, walk, REF_LIMIT); + ObjectId headId = repo.resolve(Constants.HEAD); + if (headId != null) { + RevObject head = walk.parseAny(headId); + // TODO(dborowitz): Handle non-commit or missing HEAD? + if (head.getType() == Constants.OBJ_COMMIT) { + walk.reset(); + walk.markStart((RevCommit) head); + paginator = new Paginator(walk, LOG_LIMIT, null); + } + } + if (!data.containsKey("entries")) { + data.put("entries", ImmutableList.of()); + } + List<Map<String, Object>> branches = RefServlet.getBranchesSoyData(req, REF_LIMIT); + + data.put("cloneUrl", desc.cloneUrl); + data.put("mirroredFromUrl", Strings.nullToEmpty(desc.mirroredFromUrl)); + data.put("description", Strings.nullToEmpty(desc.description)); + data.put("branches", trim(branches)); + if (branches.size() > REF_LIMIT) { + data.put("moreBranchesUrl", GitilesView.refs().copyFrom(view).toUrl()); + } + data.put("tags", trim(tags)); + data.put("hasLog", paginator != null); + if (tags.size() > REF_LIMIT) { + data.put("moreTagsUrl", GitilesView.refs().copyFrom(view).toUrl()); + } + GitilesConfig.putVariant(getAccess(req).getConfig(), "logEntry", "logEntryVariant", data); + + if (paginator != null) { + DateFormatter df = new DateFormatter(access, Format.DEFAULT); + try (OutputStream out = startRenderStreamingHtml(req, res, "gitiles.repositoryIndex", data); + Writer w = new OutputStreamWriter(out)) { + new LogSoyData(req, access, "oneline") + .renderStreaming(paginator, "HEAD", renderer, w, df); + } + } else { + renderHtml(req, res, "gitiles.repositoryIndex", data); + } + } finally { + walk.release(); + } } @Override @@ -65,56 +118,6 @@ renderJson(req, res, desc, new TypeToken<RepositoryDescription>() {}.getType()); } - @VisibleForTesting - Map<String, ?> buildData(HttpServletRequest req) throws IOException { - GitilesView view = ViewFilter.getView(req); - Repository repo = ServletUtils.getRepository(req); - GitilesAccess access = getAccess(req); - RepositoryDescription desc = access.getRepositoryDescription(); - RevWalk walk = new RevWalk(repo); - List<Map<String, Object>> tags; - Map<String, Object> data; - try { - tags = RefServlet.getTagsSoyData(req, timeCache, walk, REF_LIMIT); - ObjectId headId = repo.resolve(Constants.HEAD); - if (headId != null) { - RevObject head = walk.parseAny(headId); - if (head.getType() == Constants.OBJ_COMMIT) { - walk.reset(); - walk.markStart((RevCommit) head); - DateFormatter df = new DateFormatter(access, Format.DEFAULT); - data = new LogSoyData(req, access, "oneline") - .toSoyData(walk, LOG_LIMIT, "HEAD", null, df); - } else { - // TODO(dborowitz): Handle non-commit or missing HEAD? - data = Maps.newHashMapWithExpectedSize(7); - } - } else { - data = Maps.newHashMapWithExpectedSize(7); - } - } finally { - walk.release(); - } - if (!data.containsKey("entries")) { - data.put("entries", ImmutableList.of()); - } - List<Map<String, Object>> branches = RefServlet.getBranchesSoyData(req, REF_LIMIT); - - data.put("cloneUrl", desc.cloneUrl); - data.put("mirroredFromUrl", Strings.nullToEmpty(desc.mirroredFromUrl)); - data.put("description", Strings.nullToEmpty(desc.description)); - data.put("branches", trim(branches)); - if (branches.size() > REF_LIMIT) { - data.put("moreBranchesUrl", GitilesView.refs().copyFrom(view).toUrl()); - } - data.put("tags", trim(tags)); - if (tags.size() > REF_LIMIT) { - data.put("moreTagsUrl", GitilesView.refs().copyFrom(view).toUrl()); - } - GitilesConfig.putVariant(getAccess(req).getConfig(), "logEntry", "logEntryVariant", data); - return data; - } - private static <T> List<T> trim(List<T> list) { return list.size() > REF_LIMIT ? list.subList(0, REF_LIMIT) : list; }
diff --git a/gitiles-servlet/src/main/resources/com/google/gitiles/static/gitiles.css b/gitiles-servlet/src/main/resources/com/google/gitiles/static/gitiles.css index 25c0754..e2adfc2 100644 --- a/gitiles-servlet/src/main/resources/com/google/gitiles/static/gitiles.css +++ b/gitiles-servlet/src/main/resources/com/google/gitiles/static/gitiles.css
@@ -316,6 +316,12 @@ color: #009933; } +ol.log > li.empty:hover, ol.log > li.empty { + background: inherit; + padding: 0px; + border: 0px; +} + /* Styles for the diff detail template. */
diff --git a/gitiles-servlet/src/main/resources/com/google/gitiles/templates/LogDetail.soy b/gitiles-servlet/src/main/resources/com/google/gitiles/templates/LogDetail.soy index 71ac795..6c4de74 100644 --- a/gitiles-servlet/src/main/resources/com/google/gitiles/templates/LogDetail.soy +++ b/gitiles-servlet/src/main/resources/com/google/gitiles/templates/LogDetail.soy
@@ -23,10 +23,6 @@ * @param breadcrumbs breadcrumbs for this page. * @param? tags optional list of tags encountered when peeling this object, with * keys corresponding to gitiles.tagDetail. - * @param? logEntryVariant variant name for log entry template. - * @param entries list of log entries; see .logEntry. - * @param? nextUrl URL for the next page of results. - * @param? previousUrl URL for the previous page of results. */ {template .logDetail} {call .header data="all" /} @@ -37,40 +33,52 @@ {/foreach} {/if} -{call .logEntries data="all" /} +{call .streamingPlaceholder /} {call .footer /} {/template} + /** - * List of log entries. + * Header for list of log entries. * - * @param? logEntryVariant variant name for log entry template. - * @param? logEntryPretty base "pretty" format for the log entry template. - * @param entries list of log entries; see .logEntry. - * @param? nextUrl URL for the next page of results. + * @param? pretty base "pretty" format for the log entry template. * @param? previousUrl URL for the previous page of results. */ -{template .logEntries} +{template .logEntriesHeader} {if $previousUrl} <div class="log-nav"> <a href="{$previousUrl}">{msg desc="text for previous URL"}« Previous{/msg}</a> </div> {/if} -{if length($entries)} - <ol class="{$logEntryPretty ?: 'default'} log"> - {foreach $entry in $entries} - <li{if $previousUrl and isFirst($entry)} class="first"{/if}> - {delcall gitiles.logEntry variant="$logEntryVariant ?: 'default'" - data="$entry" /} - </li> - {/foreach} - </ol> -{else} - <p>{msg desc="informational text for when the log is empty"}No commits.{/msg}</p> -{/if} +<ol class="{$pretty ?: 'default'} log"> +{/template} + +/** + * Wrapper for a single log entry with pretty format and variant. + * + * @param firstWithPrevious whether this entry is the first in the current list, + * but also comes below a "Previous" link. + * @param variant variant name for log entry template. + * @param entry log entry; see .logEntry. + */ +{template .logEntryWrapper} +// TODO(dborowitz): Better CSS instead of this firstWithPrevious hack. +<li{if $firstWithPrevious} class="first"{/if}> + {delcall gitiles.logEntry variant="$variant ?: 'default'" data="$entry" /} +</li> +{/template} + + +/** + * Footer for the list of log entries. + * + * @param? nextUrl URL for the next page of results. + */ +{template .logEntriesFooter} +</ol> {if $nextUrl} <div class="log-nav"> <a href="{$nextUrl}">{msg desc="text for next URL"}Next »{/msg}</a> @@ -80,6 +88,14 @@ /** + * Single log entry indicating the full log is empty. + */ +{template .emptyLog} +<li class="empty">{msg desc="informational text for when the log is empty"}No commits.{/msg}</p> +{/template} + + +/** * Single pretty log entry, similar to --pretty=oneline. * * @param abbrevSha abbreviated SHA-1.
diff --git a/gitiles-servlet/src/main/resources/com/google/gitiles/templates/RepositoryIndex.soy b/gitiles-servlet/src/main/resources/com/google/gitiles/templates/RepositoryIndex.soy index 10414a5..aaba64b 100644 --- a/gitiles-servlet/src/main/resources/com/google/gitiles/templates/RepositoryIndex.soy +++ b/gitiles-servlet/src/main/resources/com/google/gitiles/templates/RepositoryIndex.soy
@@ -27,10 +27,7 @@ * @param? moreBranchesUrl URL to show more branches, if necessary. * @param tags list of tag objects with url and name keys. * @param? moreTagsUrl URL to show more branches, if necessary. - * @param? nextUrl URL for the next page of log results. - * @param? previousUrl URL for the previous page of log results. - * @param? logEntryVariant variant name for log entry template. - * @param entries list of log entries; see .logEntry. + * @param hasLog whether a log should be shown for HEAD. */ {template .repositoryIndex} {call .header} @@ -60,10 +57,10 @@ git clone {$cloneUrl} </textarea> -{if length($entries) and (length($branches) or length($tags))} +{if $hasLog and (length($branches) or length($tags))} <div class="repository-shortlog-wrapper"> <div class="repository-shortlog"> - {call .logEntries data="all" /} + {call .streamingPlaceholder /} </div> </div> @@ -71,8 +68,8 @@ {call .branches_ data="all" /} {call .tags_ data="all" /} </div> -{elseif length($entries)} - {call .logEntries data="all" /} +{elseif $hasLog} + {call .streamingPlaceholder /} {elseif length($branches) or length($tags)} {call .branches_ data="all" /} {call .tags_ data="all" /}