Split intraline differences into their own cache

The bulk of the running time for loading a PatchList entry from the
PatchListCache is actually within the intraline difference code.
Assuming no bugs, running JGit's DiffFormatter and converting the
results is typically sub-second, even for a very large change that
has renames.

By deferring the intraline difference logic until its actually
required to display a particular file allows file lists to load
very quickly, and doesn't penalize users who have disabled the
intraline difference setting in their account preferences.

This also offers a work around for the dreaded "patch of death"
situation.  When a bad file is encountered and it won't load,
users can temporarily switch off intraline difference and open the
file anyway.  Unfortunately the server will still have at least
one worker thread stuck generating the intraline difference cache
record, but at least the change is still viewable.

Change-Id: I61bbd3d068bd9b3da9d1018c0f66969a5b6ff58e
Signed-off-by: Shawn O. Pearce <sop@google.com>

9 files changed, 368 insertions, 166 deletions

diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptBuilder.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptBuilder.java
index 730d2699da..30d771602c 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptBuilder.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptBuilder.java
@@ -25,6 +25,8 @@ import com.google.gerrit.reviewdb.Patch;
 import com.google.gerrit.reviewdb.PatchLineComment;
 import com.google.gerrit.reviewdb.AccountDiffPreference.Whitespace;
 import com.google.gerrit.server.FileTypeRegistry;
+import com.google.gerrit.server.patch.IntraLineDiff;
+import com.google.gerrit.server.patch.PatchListCache;
 import com.google.gerrit.server.patch.PatchListEntry;
 import com.google.gerrit.server.patch.Text;
 import com.google.inject.Inject;
@@ -75,13 +77,15 @@ class PatchScriptBuilder {
 
   private List<Edit> edits;
   private final FileTypeRegistry registry;
+  private final PatchListCache patchListCache;
   private int context;
 
   @Inject
-  PatchScriptBuilder(final FileTypeRegistry ftr) {
+  PatchScriptBuilder(final FileTypeRegistry ftr, final PatchListCache plc) {
     a = new Side();
     b = new Side();
     registry = ftr;
+    patchListCache = plc;
   }
 
   void setRepository(final Repository r) {
@@ -110,19 +114,21 @@ class PatchScriptBuilder {
   }
 
   PatchScript toPatchScript(final PatchListEntry content,
-      final boolean intralineDifference, final CommentDetail comments,
-      final List<Patch> history) throws IOException {
+      final CommentDetail comments, final List<Patch> history)
+      throws IOException {
     reader = db.newObjectReader();
     try {
-      return build(content, intralineDifference, comments, history);
+      return build(content, comments, history);
     } finally {
       reader.release();
     }
   }
 
   private PatchScript build(final PatchListEntry content,
-      final boolean intralineDifference, final CommentDetail comments,
-      final List<Patch> history) throws IOException {
+      final CommentDetail comments, final List<Patch> history)
+      throws IOException {
+    boolean intralineDifference = diffPrefs.isIntralineDifference();
+
     a.path = oldName(content);
     b.path = newName(content);
 
@@ -130,6 +136,20 @@ class PatchScriptBuilder {
     b.resolve(a, bId);
 
     edits = new ArrayList<Edit>(content.getEdits());
+
+    if (intralineDifference) {
+      if (isModify(content)) {
+        IntraLineDiff d = patchListCache.get(a.id, a.src, b.id, b.src, edits);
+        if (d != null) {
+          edits = new ArrayList<Edit>(d.getEdits());
+        } else {
+          intralineDifference = false;
+        }
+      } else {
+        intralineDifference = false;
+      }
+    }
+
     ensureCommentsVisible(comments);
 
     boolean hugeFile = false;
@@ -173,6 +193,20 @@ class PatchScriptBuilder {
         b.displayMethod, comments, history, hugeFile, intralineDifference);
   }
 
+  private static boolean isModify(PatchListEntry content) {
+    switch (content.getChangeType()) {
+      case MODIFIED:
+      case COPIED:
+      case RENAMED:
+        return true;
+
+      case ADDED:
+      case DELETED:
+      default:
+        return false;
+    }
+  }
+
   private static String oldName(final PatchListEntry entry) {
     switch (entry.getChangeType()) {
       case ADDED:
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java
index d606577a3d..377cf49c5b 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java
@@ -144,7 +144,6 @@ class PatchScriptFactory extends Handler<PatchScript> {
     }
     try {
       final PatchList list = listFor(keyFor(diffPrefs.getIgnoreWhitespace()));
-      final boolean intraline = list.hasIntralineDifference();
       final PatchScriptBuilder b = newBuilder(list, git);
       final PatchListEntry content = list.get(patchKey.getFileName());
 
@@ -153,7 +152,7 @@ class PatchScriptFactory extends Handler<PatchScript> {
           content.getNewName());
 
       try {
-        return b.toPatchScript(content, intraline, comments, history);
+        return b.toPatchScript(content, comments, history);
       } catch (IOException e) {
         log.error("File content unavailable", e);
         throw new NoSuchChangeException(changeId, e);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineDiff.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineDiff.java
new file mode 100644
index 0000000000..f8f339e2e4
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineDiff.java
@@ -0,0 +1,97 @@
+// Copyright (C) 2010 The Android Open Source Project
+//
+// 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.gerrit.server.patch;
+
+import static com.google.gerrit.server.ioutil.BasicSerialization.readVarInt32;
+import static com.google.gerrit.server.ioutil.BasicSerialization.writeVarInt32;
+
+import org.eclipse.jgit.diff.Edit;
+import org.eclipse.jgit.diff.ReplaceEdit;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.OutputStream;
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+public class IntraLineDiff implements Serializable {
+  static final long serialVersionUID = IntraLineDiffKey.serialVersionUID;
+
+  private List<Edit> edits;
+
+  IntraLineDiff(List<Edit> edits) {
+    this.edits = edits;
+  }
+
+  public List<Edit> getEdits() {
+    return edits;
+  }
+
+  private void writeObject(final ObjectOutputStream out) throws IOException {
+    writeVarInt32(out, edits.size());
+    for (Edit e : edits) {
+      writeEdit(out, e);
+
+      if (e instanceof ReplaceEdit) {
+        ReplaceEdit r = (ReplaceEdit) e;
+        writeVarInt32(out, r.getInternalEdits().size());
+        for (Edit i : r.getInternalEdits()) {
+          writeEdit(out, i);
+        }
+      } else {
+        writeVarInt32(out, 0);
+      }
+    }
+  }
+
+  private void readObject(final ObjectInputStream in) throws IOException {
+    int editCount = readVarInt32(in);
+    Edit[] editArray = new Edit[editCount];
+    for (int i = 0; i < editCount; i++) {
+      editArray[i] = readEdit(in);
+
+      int innerCount = readVarInt32(in);
+      if (0 < innerCount) {
+        Edit[] inner = new Edit[innerCount];
+        for (int j = 0; j < innerCount; j++)
+          inner[j] = readEdit(in);
+        editArray[i] = new ReplaceEdit(editArray[i], toList(inner));
+      }
+    }
+  }
+
+  private static void writeEdit(OutputStream out, Edit e) throws IOException {
+    writeVarInt32(out, e.getBeginA());
+    writeVarInt32(out, e.getEndA());
+    writeVarInt32(out, e.getBeginB());
+    writeVarInt32(out, e.getEndB());
+  }
+
+  private static Edit readEdit(InputStream in) throws IOException {
+    int beginA = readVarInt32(in);
+    int endA = readVarInt32(in);
+    int beginB = readVarInt32(in);
+    int endB = readVarInt32(in);
+    return new Edit(beginA, endA, beginB, endB);
+  }
+
+  private static List<Edit> toList(Edit[] l) {
+    return Collections.unmodifiableList(Arrays.asList(l));
+  }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineDiffKey.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineDiffKey.java
new file mode 100644
index 0000000000..899dda5b1c
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineDiffKey.java
@@ -0,0 +1,103 @@
+// Copyright (C) 2010 The Android Open Source Project
+//
+// 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.gerrit.server.patch;
+
+import static org.eclipse.jgit.lib.ObjectIdSerialization.readNotNull;
+import static org.eclipse.jgit.lib.ObjectIdSerialization.writeNotNull;
+
+import org.eclipse.jgit.diff.Edit;
+import org.eclipse.jgit.lib.ObjectId;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.util.List;
+
+public class IntraLineDiffKey implements Serializable {
+  static final long serialVersionUID = 1L;
+
+  private transient ObjectId aId;
+  private transient ObjectId bId;
+
+  // Transient data passed through on cache misses to the loader.
+
+  private transient Text aText;
+  private transient Text bText;
+  private transient List<Edit> edits;
+
+  IntraLineDiffKey(ObjectId aId, Text aText, ObjectId bId, Text bText,
+      List<Edit> edits) {
+    this.aId = aId;
+    this.bId = bId;
+
+    this.aText = aText;
+    this.bText = bText;
+    this.edits = edits;
+  }
+
+  Text getTextA() {
+    return aText;
+  }
+
+  Text getTextB() {
+    return bText;
+  }
+
+  List<Edit> getEdits() {
+    return edits;
+  }
+
+  @Override
+  public int hashCode() {
+    int h = 0;
+
+    h = h * 31 + aId.hashCode();
+    h = h * 31 + bId.hashCode();
+
+    return h;
+  }
+
+  @Override
+  public boolean equals(final Object o) {
+    if (o instanceof IntraLineDiffKey) {
+      final IntraLineDiffKey k = (IntraLineDiffKey) o;
+      return aId.equals(k.aId) //
+          && bId.equals(k.bId);
+    }
+    return false;
+  }
+
+  @Override
+  public String toString() {
+    StringBuilder n = new StringBuilder();
+    n.append("IntraLineDiffKey[");
+    n.append(aId.name());
+    n.append("..");
+    n.append(bId.name());
+    n.append("]");
+    return n.toString();
+  }
+
+  private void writeObject(final ObjectOutputStream out) throws IOException {
+    writeNotNull(out, aId);
+    writeNotNull(out, bId);
+  }
+
+  private void readObject(final ObjectInputStream in) throws IOException {
+    aId = readNotNull(in);
+    bId = readNotNull(in);
+  }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
index 6e68f27f2e..7194a22ae4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
@@ -59,18 +59,15 @@ public class PatchList implements Serializable {
   @Nullable
   private transient ObjectId oldId;
   private transient ObjectId newId;
-  private transient boolean intralineDifference;
   private transient boolean againstParent;
   private transient int insertions;
   private transient int deletions;
   private transient PatchListEntry[] patches;
 
   PatchList(@Nullable final AnyObjectId oldId, final AnyObjectId newId,
-      final boolean intralineDifference, final boolean againstParent,
-      final PatchListEntry[] patches) {
+      final boolean againstParent, final PatchListEntry[] patches) {
     this.oldId = oldId != null ? oldId.copy() : null;
     this.newId = newId.copy();
-    this.intralineDifference = intralineDifference;
     this.againstParent = againstParent;
 
     // We assume index 0 contains the magic commit message entry.
@@ -101,11 +98,6 @@ public class PatchList implements Serializable {
     return Collections.unmodifiableList(Arrays.asList(patches));
   }
 
-  /** @return true if this list was computed with intraline difference enabled. */
-  public boolean hasIntralineDifference() {
-    return intralineDifference;
-  }
-
   /** @return true if {@link #getOldId} is {@link #getNewId}'s ancestor. */
   public boolean isAgainstParent() {
     return againstParent;
@@ -171,7 +163,6 @@ public class PatchList implements Serializable {
     try {
       writeCanBeNull(out, oldId);
       writeNotNull(out, newId);
-      writeVarInt32(out, intralineDifference ? 1 : 0);
       writeVarInt32(out, againstParent ? 1 : 0);
       writeVarInt32(out, insertions);
       writeVarInt32(out, deletions);
@@ -191,7 +182,6 @@ public class PatchList implements Serializable {
     try {
       oldId = readCanBeNull(in);
       newId = readNotNull(in);
-      intralineDifference = readVarInt32(in) != 0;
       againstParent = readVarInt32(in) != 0;
       insertions = readVarInt32(in);
       deletions = readVarInt32(in);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
index 1a6d2ae1ed..c679f4af80 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
@@ -18,6 +18,11 @@ import com.google.gerrit.reviewdb.Change;
 import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gerrit.reviewdb.AccountDiffPreference.Whitespace;
 
+import org.eclipse.jgit.diff.Edit;
+import org.eclipse.jgit.lib.ObjectId;
+
+import java.util.List;
+
 /** Provides a cached list of {@link PatchListEntry}. */
 public interface PatchListCache {
   public PatchList get(PatchListKey key);
@@ -25,4 +30,7 @@ public interface PatchListCache {
   public PatchList get(Change change, PatchSet patchSet);
 
   public PatchList get(Change change, PatchSet patchSet, Whitespace whitespace);
+
+  public IntraLineDiff get(ObjectId aId, Text aText, ObjectId bId, Text bText,
+      List<Edit> edits);
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
index ec1ec094bc..087ed51eb6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
@@ -87,13 +87,11 @@ import org.eclipse.jgit.diff.MyersDiff;
 import org.eclipse.jgit.diff.RawText;
 import org.eclipse.jgit.diff.RawTextComparator;
 import org.eclipse.jgit.diff.ReplaceEdit;
-import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.patch.FileHeader;
@@ -115,7 +113,8 @@ import java.util.regex.Pattern;
 /** Provides a cached list of {@link PatchListEntry}. */
 @Singleton
 public class PatchListCacheImpl implements PatchListCache {
-  private static final String CACHE_NAME = "diff";
+  private static final String FILE_NAME = "diff";
+  private static final String INTRA_NAME = "diff_intraline";
 
   private static final Pattern BLANK_LINE_RE =
       Pattern.compile("^[ \\t]*(|[{}]|/\\*\\*?|\\*)[ \\t]*$");
@@ -126,29 +125,45 @@ public class PatchListCacheImpl implements PatchListCache {
     return new CacheModule() {
       @Override
       protected void configure() {
-        final TypeLiteral<Cache<PatchListKey, PatchList>> type =
+        final TypeLiteral<Cache<PatchListKey, PatchList>> fileType =
             new TypeLiteral<Cache<PatchListKey, PatchList>>() {};
-        disk(type, CACHE_NAME) //
+        disk(fileType, FILE_NAME) //
             .memoryLimit(128) // very large items, cache only a few
             .evictionPolicy(EvictionPolicy.LRU) // prefer most recent
-            .populateWith(Loader.class) //
+            .populateWith(PatchListLoader.class) //
         ;
+
+        final TypeLiteral<Cache<IntraLineDiffKey, IntraLineDiff>> intraType =
+            new TypeLiteral<Cache<IntraLineDiffKey, IntraLineDiff>>() {};
+        disk(intraType, INTRA_NAME) //
+            .memoryLimit(128) // very large items, cache only a few
+            .evictionPolicy(EvictionPolicy.LRU) // prefer most recent
+            .populateWith(IntraLineLoader.class) //
+        ;
+
         bind(PatchListCacheImpl.class);
         bind(PatchListCache.class).to(PatchListCacheImpl.class);
       }
     };
   }
 
-  private final Cache<PatchListKey, PatchList> cache;
+  private final Cache<PatchListKey, PatchList> fileCache;
+  private final Cache<IntraLineDiffKey, IntraLineDiff> intraCache;
+  private final boolean computeIntraline;
 
   @Inject
   PatchListCacheImpl(
-      @Named(CACHE_NAME) final Cache<PatchListKey, PatchList> thecache) {
-    cache = thecache;
+      @Named(FILE_NAME) final Cache<PatchListKey, PatchList> fileCache,
+      @Named(INTRA_NAME) final Cache<IntraLineDiffKey, IntraLineDiff> intraCache,
+      @GerritServerConfig Config cfg) {
+    this.fileCache = fileCache;
+    this.intraCache = intraCache;
+
+    this.computeIntraline = cfg.getBoolean("cache", "diff", "intraline", true);
   }
 
   public PatchList get(final PatchListKey key) {
-    return cache.get(key);
+    return fileCache.get(key);
   }
 
   public PatchList get(final Change change, final PatchSet patchSet) {
@@ -163,14 +178,41 @@ public class PatchListCacheImpl implements PatchListCache {
     return get(new PatchListKey(projectKey, a, b, whitespace));
   }
 
-  static class Loader extends EntryCreator<PatchListKey, PatchList> {
+  @Override
+  public IntraLineDiff get(ObjectId aId, Text aText, ObjectId bId, Text bText,
+      List<Edit> edits) {
+    if (computeIntraline) {
+      IntraLineDiffKey key =
+          new IntraLineDiffKey(aId, aText, bId, bText, edits);
+      return intraCache.get(key);
+    } else {
+      return null;
+    }
+  }
+
+  private static RawTextComparator comparatorFor(Whitespace ws) {
+    switch (ws) {
+      case IGNORE_ALL_SPACE:
+        return RawTextComparator.WS_IGNORE_ALL;
+
+      case IGNORE_SPACE_AT_EOL:
+        return RawTextComparator.WS_IGNORE_TRAILING;
+
+      case IGNORE_SPACE_CHANGE:
+        return RawTextComparator.WS_IGNORE_CHANGE;
+
+      case IGNORE_NONE:
+      default:
+        return RawTextComparator.DEFAULT;
+    }
+  }
+
+  static class PatchListLoader extends EntryCreator<PatchListKey, PatchList> {
     private final GitRepositoryManager repoManager;
-    private final boolean computeIntraline;
 
     @Inject
-    Loader(GitRepositoryManager mgr, @GerritServerConfig Config config) {
+    PatchListLoader(GitRepositoryManager mgr) {
       repoManager = mgr;
-      computeIntraline = config.getBoolean("cache", "diff", "intraline", true);
     }
 
     @Override
@@ -187,23 +229,7 @@ public class PatchListCacheImpl implements PatchListCache {
         final Repository repo) throws IOException {
       // TODO(jeffschu) correctly handle merge commits
 
-      RawTextComparator cmp;
-      switch (key.getWhitespace()) {
-        case IGNORE_ALL_SPACE:
-          cmp = RawTextComparator.WS_IGNORE_ALL;
-          break;
-        case IGNORE_SPACE_AT_EOL:
-          cmp = RawTextComparator.WS_IGNORE_TRAILING;
-          break;
-        case IGNORE_SPACE_CHANGE:
-          cmp = RawTextComparator.WS_IGNORE_CHANGE;
-          break;
-        case IGNORE_NONE:
-        default:
-          cmp = RawTextComparator.DEFAULT;
-          break;
-      }
-
+      final RawTextComparator cmp = comparatorFor(key.getWhitespace());
       final ObjectReader reader = repo.newObjectReader();
       try {
         final RevWalk rw = new RevWalk(reader);
@@ -215,7 +241,7 @@ public class PatchListCacheImpl implements PatchListCache {
           //
           final PatchListEntry[] entries = new PatchListEntry[1];
           entries[0] = newCommitMessage(cmp, repo, reader, null, b);
-          return new PatchList(a, b, computeIntraline, true, entries);
+          return new PatchList(a, b, true, entries);
         }
 
         final boolean againstParent =
@@ -254,18 +280,17 @@ public class PatchListCacheImpl implements PatchListCache {
             againstParent ? null : aCommit, b);
         for (int i = 0; i < cnt; i++) {
           FileHeader fh = df.toFileHeader(diffEntries.get(i));
-          entries[1 + i] = newEntry(reader, aTree, bTree, fh);
+          entries[1 + i] = newEntry(aTree, fh);
         }
-        return new PatchList(a, b, computeIntraline, againstParent, entries);
+        return new PatchList(a, b, againstParent, entries);
       } finally {
         reader.release();
       }
     }
 
-    private PatchListEntry newCommitMessage(
-        final RawTextComparator cmp, final Repository db,
-        final ObjectReader reader, final RevCommit aCommit,
-        final RevCommit bCommit) throws IOException {
+    private PatchListEntry newCommitMessage(final RawTextComparator cmp,
+        final Repository db, final ObjectReader reader,
+        final RevCommit aCommit, final RevCommit bCommit) throws IOException {
       StringBuilder hdr = new StringBuilder();
 
       hdr.append("diff --git");
@@ -284,7 +309,8 @@ public class PatchListCacheImpl implements PatchListCache {
       }
       hdr.append("+++ b/" + Patch.COMMIT_MSG + "\n");
 
-      Text aText = aCommit != null ? Text.forCommit(db, reader, aCommit) : Text.EMPTY;
+      Text aText =
+          aCommit != null ? Text.forCommit(db, reader, aCommit) : Text.EMPTY;
       Text bText = Text.forCommit(db, reader, bCommit);
 
       byte[] rawHdr = hdr.toString().getBytes("UTF-8");
@@ -292,11 +318,10 @@ public class PatchListCacheImpl implements PatchListCache {
       RawText bRawText = new RawText(bText.getContent());
       EditList edits = new HistogramDiff().diff(cmp, aRawText, bRawText);
       FileHeader fh = new FileHeader(rawHdr, edits, PatchType.UNIFIED);
-      return newEntry(reader, aText, bText, edits, null, null, fh);
+      return new PatchListEntry(fh, edits);
     }
 
-    private PatchListEntry newEntry(ObjectReader reader, RevTree aTree,
-        RevTree bTree, FileHeader fileHeader) throws IOException {
+    private PatchListEntry newEntry(RevTree aTree, FileHeader fileHeader) {
       final FileMode oldMode = fileHeader.getOldMode();
       final FileMode newMode = fileHeader.getNewMode();
 
@@ -313,36 +338,57 @@ public class PatchListCacheImpl implements PatchListCache {
       List<Edit> edits = fileHeader.toEditList();
       if (edits.isEmpty()) {
         return new PatchListEntry(fileHeader, Collections.<Edit> emptyList());
-      }
-      if (!computeIntraline) {
+      } else {
         return new PatchListEntry(fileHeader, edits);
       }
+    }
 
-      switch (fileHeader.getChangeType()) {
-        case ADD:
-        case DELETE:
-          return new PatchListEntry(fileHeader, edits);
+    private static RevObject aFor(final PatchListKey key,
+        final Repository repo, final RevWalk rw, final RevCommit b)
+        throws IOException {
+      if (key.getOldId() != null) {
+        return rw.parseAny(key.getOldId());
+      }
+
+      switch (b.getParentCount()) {
+        case 0:
+          return rw.parseAny(emptyTree(repo));
+        case 1: {
+          RevCommit r = b.getParent(0);
+          rw.parseBody(r);
+          return r;
+        }
+        default:
+          // merge commit, return null to force combined diff behavior
+          return null;
       }
+    }
 
-      return newEntry(reader, null, null, edits, aTree, bTree, fileHeader);
+    private static ObjectId emptyTree(final Repository repo) throws IOException {
+      ObjectInserter oi = repo.newObjectInserter();
+      try {
+        ObjectId id = oi.insert(Constants.OBJ_TREE, new byte[] {});
+        oi.flush();
+        return id;
+      } finally {
+        oi.release();
+      }
     }
+  }
+
+  static class IntraLineLoader extends
+      EntryCreator<IntraLineDiffKey, IntraLineDiff> {
+    @Override
+    public IntraLineDiff createEntry(IntraLineDiffKey key) throws Exception {
+      List<Edit> edits = new ArrayList<Edit>(key.getEdits());
+      Text aContent = key.getTextA();
+      Text bContent = key.getTextB();
+      combineLineEdits(edits, aContent, bContent);
 
-    private PatchListEntry newEntry(ObjectReader reader, Text aContent,
-        Text bContent, List<Edit> edits, RevTree aTree, RevTree bTree,
-        FileHeader fileHeader) throws IOException {
       for (int i = 0; i < edits.size(); i++) {
         Edit e = edits.get(i);
 
         if (e.getType() == Edit.Type.REPLACE) {
-          if (aContent == null) {
-            edits = new ArrayList<Edit>(edits);
-            aContent = read(reader, fileHeader.getOldPath(), aTree);
-            bContent = read(reader, fileHeader.getNewPath(), bTree);
-            combineLineEdits(edits, aContent, bContent);
-            i = -1; // restart the entire scan after combining lines.
-            continue;
-          }
-
           CharText a = new CharText(aContent, e.getBeginA(), e.getEndA());
           CharText b = new CharText(bContent, e.getBeginB(), e.getEndB());
           CharTextComparator cmp = new CharTextComparator();
@@ -520,7 +566,7 @@ public class PatchListCacheImpl implements PatchListCache {
         }
       }
 
-      return new PatchListEntry(fileHeader, edits);
+      return new IntraLineDiff(edits);
     }
 
     private static void combineLineEdits(List<Edit> edits, Text a, Text b) {
@@ -594,52 +640,5 @@ public class PatchListCacheImpl implements PatchListCache {
       }
       return b < e;
     }
-
-    private static Text read(ObjectReader reader, String path, RevTree tree)
-        throws IOException {
-      TreeWalk tw = TreeWalk.forPath(reader, path, tree);
-      if (tw == null || tw.getFileMode(0).getObjectType() != Constants.OBJ_BLOB) {
-        return Text.EMPTY;
-      }
-      ObjectLoader ldr;
-      try {
-        ldr = reader.open(tw.getObjectId(0), Constants.OBJ_BLOB);
-      } catch (MissingObjectException notFound) {
-        return Text.EMPTY;
-      }
-      return new Text(ldr);
-    }
-
-    private static RevObject aFor(final PatchListKey key,
-        final Repository repo, final RevWalk rw, final RevCommit b)
-        throws IOException {
-      if (key.getOldId() != null) {
-        return rw.parseAny(key.getOldId());
-      }
-
-      switch (b.getParentCount()) {
-        case 0:
-          return rw.parseAny(emptyTree(repo));
-        case 1:{
-          RevCommit r = b.getParent(0);
-          rw.parseBody(r);
-          return r;
-        }
-        default:
-          // merge commit, return null to force combined diff behavior
-          return null;
-      }
-    }
-
-    private static ObjectId emptyTree(final Repository repo) throws IOException {
-      ObjectInserter oi = repo.newObjectInserter();
-      try {
-        ObjectId id = oi.insert(Constants.OBJ_TREE, new byte[] {});
-        oi.flush();
-        return id;
-      } finally {
-        oi.release();
-      }
-    }
   }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
index fac8ec4350..837ec9c448 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
@@ -29,7 +29,6 @@ import com.google.gerrit.reviewdb.Patch.ChangeType;
 import com.google.gerrit.reviewdb.Patch.PatchType;
 
 import org.eclipse.jgit.diff.Edit;
-import org.eclipse.jgit.diff.ReplaceEdit;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.patch.CombinedFileHeader;
@@ -183,27 +182,13 @@ public class PatchListEntry {
 
     writeVarInt32(out, edits.size());
     for (final Edit e : edits) {
-      write(out, e);
-
-      if (e instanceof ReplaceEdit) {
-        ReplaceEdit r = (ReplaceEdit) e;
-        writeVarInt32(out, r.getInternalEdits().size());
-        for (Edit i : r.getInternalEdits()) {
-          write(out, i);
-        }
-      } else {
-        writeVarInt32(out, 0);
-      }
+      writeVarInt32(out, e.getBeginA());
+      writeVarInt32(out, e.getEndA());
+      writeVarInt32(out, e.getBeginB());
+      writeVarInt32(out, e.getEndB());
     }
   }
 
-  private void write(final OutputStream out, final Edit e) throws IOException {
-    writeVarInt32(out, e.getBeginA());
-    writeVarInt32(out, e.getEndA());
-    writeVarInt32(out, e.getBeginB());
-    writeVarInt32(out, e.getEndB());
-  }
-
   static PatchListEntry readFrom(final InputStream in) throws IOException {
     final ChangeType changeType = readEnum(in, ChangeType.values());
     final PatchType patchType = readEnum(in, PatchType.values());
@@ -216,16 +201,11 @@ public class PatchListEntry {
     final int editCount = readVarInt32(in);
     final Edit[] editArray = new Edit[editCount];
     for (int i = 0; i < editCount; i++) {
-      editArray[i] = readEdit(in);
-
-      int innerCount = readVarInt32(in);
-      if (0 < innerCount) {
-        Edit[] inner = new Edit[innerCount];
-        for (int innerIdx = 0; innerIdx < innerCount; innerIdx++) {
-          inner[innerIdx] = readEdit(in);
-        }
-        editArray[i] = new ReplaceEdit(editArray[i], toList(inner));
-      }
+      int beginA = readVarInt32(in);
+      int endA = readVarInt32(in);
+      int beginB = readVarInt32(in);
+      int endB = readVarInt32(in);
+      editArray[i] = new Edit(beginA, endA, beginB, endB);
     }
 
     return new PatchListEntry(changeType, patchType, oldName, newName, hdr,
@@ -236,14 +216,6 @@ public class PatchListEntry {
     return Collections.unmodifiableList(Arrays.asList(l));
   }
 
-  private static Edit readEdit(final InputStream in) throws IOException {
-    final int beginA = readVarInt32(in);
-    final int endA = readVarInt32(in);
-    final int beginB = readVarInt32(in);
-    final int endB = readVarInt32(in);
-    return new Edit(beginA, endA, beginB, endB);
-  }
-
   private static byte[] compact(final FileHeader h) {
     final int end = end(h);
     if (h.getStartOffset() == 0 && end == h.getBuffer().length) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java
index 715b90ae40..18fa5b045a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java
@@ -35,7 +35,7 @@ import java.io.Serializable;
 import javax.annotation.Nullable;
 
 public class PatchListKey implements Serializable {
-  static final long serialVersionUID = 15L;
+  static final long serialVersionUID = 16L;
 
   private transient ObjectId oldId;
   private transient ObjectId newId;