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
This commit is contained in:
Shawn Pearce 2013-03-14 16:14:04 -07:00
parent fc6b898cbe
commit b2c0021b8a
1 changed files with 7 additions and 18 deletions

View File

@ -100,12 +100,6 @@ public class DfsGarbageCollector {
private Set<ObjectId> nonHeads; private Set<ObjectId> nonHeads;
/** Sum of object counts in {@link #packsBefore}. */
private long objectsBefore;
/** Sum of object counts iN {@link #newPackDesc}. */
private long objectsPacked;
private Set<ObjectId> tagTargets; private Set<ObjectId> tagTargets;
/** /**
@ -288,7 +282,7 @@ private void packHeads(ProgressMonitor pm) throws IOException {
} }
private void packRest(ProgressMonitor pm) throws IOException { private void packRest(ProgressMonitor pm) throws IOException {
if (nonHeads.isEmpty() || objectsPacked == getObjectsBefore()) if (nonHeads.isEmpty())
return; return;
PackWriter pw = newPackWriter(); PackWriter pw = newPackWriter();
@ -304,14 +298,11 @@ private void packRest(ProgressMonitor pm) throws IOException {
} }
private void packGarbage(ProgressMonitor pm) throws IOException { private void packGarbage(ProgressMonitor pm) throws IOException {
if (objectsPacked == getObjectsBefore())
return;
// TODO(sop) This is ugly. The garbage pack needs to be deleted. // TODO(sop) This is ugly. The garbage pack needs to be deleted.
PackWriter pw = newPackWriter(); PackWriter pw = newPackWriter();
try { try {
RevWalk pool = new RevWalk(ctx); RevWalk pool = new RevWalk(ctx);
pm.beginTask("Finding garbage", (int) getObjectsBefore()); pm.beginTask("Finding garbage", objectsBefore());
for (DfsPackFile oldPack : packsBefore) { for (DfsPackFile oldPack : packsBefore) {
PackIndex oldIdx = oldPack.getPackIndex(ctx); PackIndex oldIdx = oldPack.getPackIndex(ctx);
for (PackIndex.MutableEntry ent : oldIdx) { for (PackIndex.MutableEntry ent : oldIdx) {
@ -343,12 +334,11 @@ private static boolean isHead(Ref ref) {
return ref.getName().startsWith(Constants.R_HEADS); return ref.getName().startsWith(Constants.R_HEADS);
} }
private long getObjectsBefore() { private int objectsBefore() {
if (objectsBefore == 0) { int cnt = 0;
for (DfsPackFile p : packsBefore) for (DfsPackFile p : packsBefore)
objectsBefore += p.getPackDescription().getObjectCount(); cnt += p.getPackDescription().getObjectCount();
} return cnt;
return objectsBefore;
} }
private PackWriter newPackWriter() { private PackWriter newPackWriter() {
@ -403,7 +393,6 @@ public boolean contains(AnyObjectId objectId) {
PackWriter.Statistics stats = pw.getStatistics(); PackWriter.Statistics stats = pw.getStatistics();
pack.setPackStats(stats); pack.setPackStats(stats);
objectsPacked += stats.getTotalObjects();
newPackStats.add(stats); newPackStats.add(stats);
DfsBlockCache.getInstance().getOrCreate(pack, null); DfsBlockCache.getInstance().getOrCreate(pack, null);