From b2c0021b8ae7c45dfd351b127bb674d3ba20026b Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 14 Mar 2013 16:14:04 -0700 Subject: [PATCH] Remove objects before optimization from DfsGarbageCollector Just counting objects is not sufficient. There are some race conditions with receive packs and delta base completion that may confuse such a simple algorithm. Instead always do the larger set computations, and rely on the PackWriter having no objects pending as the way to avoid creating an empty pack file. Change-Id: Ic81fefb158ed6ef8d6522062f2be0338a49f6bc4 --- .../jgit/storage/dfs/DfsGarbageCollector.java | 25 ++++++------------- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsGarbageCollector.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsGarbageCollector.java index 40555dde1..8f175a822 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsGarbageCollector.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsGarbageCollector.java @@ -100,12 +100,6 @@ public class DfsGarbageCollector { private Set nonHeads; - /** Sum of object counts in {@link #packsBefore}. */ - private long objectsBefore; - - /** Sum of object counts iN {@link #newPackDesc}. */ - private long objectsPacked; - private Set tagTargets; /** @@ -288,7 +282,7 @@ public class DfsGarbageCollector { } private void packRest(ProgressMonitor pm) throws IOException { - if (nonHeads.isEmpty() || objectsPacked == getObjectsBefore()) + if (nonHeads.isEmpty()) return; PackWriter pw = newPackWriter(); @@ -304,14 +298,11 @@ public class DfsGarbageCollector { } private void packGarbage(ProgressMonitor pm) throws IOException { - if (objectsPacked == getObjectsBefore()) - return; - // TODO(sop) This is ugly. The garbage pack needs to be deleted. PackWriter pw = newPackWriter(); try { RevWalk pool = new RevWalk(ctx); - pm.beginTask("Finding garbage", (int) getObjectsBefore()); + pm.beginTask("Finding garbage", objectsBefore()); for (DfsPackFile oldPack : packsBefore) { PackIndex oldIdx = oldPack.getPackIndex(ctx); for (PackIndex.MutableEntry ent : oldIdx) { @@ -343,12 +334,11 @@ public class DfsGarbageCollector { return ref.getName().startsWith(Constants.R_HEADS); } - private long getObjectsBefore() { - if (objectsBefore == 0) { - for (DfsPackFile p : packsBefore) - objectsBefore += p.getPackDescription().getObjectCount(); - } - return objectsBefore; + private int objectsBefore() { + int cnt = 0; + for (DfsPackFile p : packsBefore) + cnt += p.getPackDescription().getObjectCount(); + return cnt; } private PackWriter newPackWriter() { @@ -403,7 +393,6 @@ public class DfsGarbageCollector { PackWriter.Statistics stats = pw.getStatistics(); pack.setPackStats(stats); - objectsPacked += stats.getTotalObjects(); newPackStats.add(stats); DfsBlockCache.getInstance().getOrCreate(pack, null);