From 751c329b356f43c082e08246c85f78eea43ff7a2 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 28 Feb 2011 15:39:31 -0800 Subject: [PATCH] PackWriter: Don't reuse commit or tag deltas JGit doesn't generate deltas for commit or tag objects when it packs a repository from scratch. This is an explicit design decision that is (mostly) justified by the fact that these objects do not delta compress well. Annotated tags are made once on stable points of the project history, it is unlikely they will ever appear again with sufficient common text to justify using a delta over just deflating the raw content. JGit never tries to delta compress annotated tags and I take the stance that these are best stored as non-deltas given how frequently they might be accessed by repository viewers. Commits only have sufficient common text when they are cherry-picked to forward-port or back-port a change from one branch to another. Even in these cases the distance between the commits as returned by the log traversal has to be small enough that they would both appear in the delta search window at the same time in order to delta compress one of the messages against the other. JGit never tries to delta compress commits, as it requires a lot of CPU time but typically does not produce a smaller pack file. Avoid reusing deltas for either of these types when constructing a new pack. To avoid killing performance during serving of network clients, UploadPack disables this code change by allowing PackWriter to reuse delta commits. Repositories that were already repacked by C Git will not have their delta commits decompressed and recompressed on the fly during object writing, saving server-side CPU resources. Change-Id: I749407e7c5c677e05e4d054b40db7656cfa7fca8 Signed-off-by: Shawn O. Pearce --- .../eclipse/jgit/storage/pack/PackWriter.java | 40 ++++++++++++++++++- .../eclipse/jgit/transport/UploadPack.java | 1 + 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java index eee121555..e9eb366f6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java @@ -175,6 +175,8 @@ public class PackWriter { private boolean reuseDeltas; + private boolean reuseDeltaCommits; + private boolean thin; private boolean useCachedPacks; @@ -275,6 +277,27 @@ public void setDeltaBaseAsOffset(boolean deltaBaseAsOffset) { this.deltaBaseAsOffset = deltaBaseAsOffset; } + /** + * Check if the writer will reuse commits that are already stored as deltas. + * + * @return true if the writer would reuse commits stored as deltas, assuming + * delta reuse is already enabled. + */ + public boolean isReuseDeltaCommits() { + return reuseDeltaCommits; + } + + /** + * Set the writer to reuse existing delta versions of commits. + * + * @param reuse + * if true, the writer will reuse any commits stored as deltas. + * By default the writer does not reuse delta commits. + */ + public void setReuseDeltaCommits(boolean reuse) { + reuseDeltaCommits = reuse; + } + /** @return true if this writer is producing a thin pack. */ public boolean isThin() { return thin; @@ -1482,7 +1505,7 @@ else if (PACK_DELTA < nFmt && otp.isDeltaRepresentation()) } else nWeight = next.getWeight(); - if (nFmt == PACK_DELTA && reuseDeltas) { + if (nFmt == PACK_DELTA && reuseDeltas && reuseDeltaFor(otp)) { ObjectId baseId = next.getDeltaBase(); ObjectToPack ptr = objectsMap.get(baseId); if (ptr != null && !ptr.isEdge()) { @@ -1509,6 +1532,21 @@ else if (PACK_DELTA < nFmt && otp.isDeltaRepresentation()) otp.select(next); } + private boolean reuseDeltaFor(ObjectToPack otp) { + switch (otp.getType()) { + case Constants.OBJ_COMMIT: + return reuseDeltaCommits; + case Constants.OBJ_TREE: + return true; + case Constants.OBJ_BLOB: + return true; + case Constants.OBJ_TAG: + return false; + default: + return true; + } + } + /** Summary of how PackWriter created the pack. */ public static class Statistics { Set interestingObjects; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index b7bf08c38..3e324d093 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -692,6 +692,7 @@ private void sendPack() throws IOException { final PackWriter pw = new PackWriter(cfg, walk.getObjectReader()); try { pw.setUseCachedPacks(true); + pw.setReuseDeltaCommits(true); pw.setDeltaBaseAsOffset(options.contains(OPTION_OFS_DELTA)); pw.setThin(options.contains(OPTION_THIN_PACK));