* GC.tooManyLooseObjects() always responded true since the loop missed to advance the iterator so it always incremented until the threshold was exceeded. * Also fix loop exit criterion which was off by 1. * Add some tests. Change-Id: I70976dfaa026efbcf3c46bd45941f37277a18e04 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>tags/v4.6.0.201612231935-r
@@ -0,0 +1,87 @@ | |||
/* | |||
* Copyright (C) 2016, Matthias Sohn <matthias.sohn@sap.com> | |||
* and other copyright owners as documented in the project's IP log. | |||
* | |||
* This program and the accompanying materials are made available | |||
* under the terms of the Eclipse Distribution License v1.0 which | |||
* accompanies this distribution, is reproduced below, and is | |||
* available at http://www.eclipse.org/org/documents/edl-v10.php | |||
* | |||
* All rights reserved. | |||
* | |||
* Redistribution and use in source and binary forms, with or | |||
* without modification, are permitted provided that the following | |||
* conditions are met: | |||
* | |||
* - Redistributions of source code must retain the above copyright | |||
* notice, this list of conditions and the following disclaimer. | |||
* | |||
* - Redistributions in binary form must reproduce the above | |||
* copyright notice, this list of conditions and the following | |||
* disclaimer in the documentation and/or other materials provided | |||
* with the distribution. | |||
* | |||
* - Neither the name of the Eclipse Foundation, Inc. nor the | |||
* names of its contributors may be used to endorse or promote | |||
* products derived from this software without specific prior | |||
* written permission. | |||
* | |||
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND | |||
* CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, | |||
* INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES | |||
* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE | |||
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR | |||
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, | |||
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT | |||
* NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; | |||
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER | |||
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, | |||
* STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) | |||
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF | |||
* ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | |||
*/ | |||
package org.eclipse.jgit.internal.storage.file; | |||
import static org.junit.Assert.assertFalse; | |||
import static org.junit.Assert.assertTrue; | |||
import org.eclipse.jgit.lib.ConfigConstants; | |||
import org.eclipse.jgit.storage.file.FileBasedConfig; | |||
import org.eclipse.jgit.test.resources.SampleDataRepositoryTestCase; | |||
import org.junit.Test; | |||
public class AutoGcTest extends GcTestCase { | |||
@Test | |||
public void testNotTooManyLooseObjects() { | |||
assertFalse("should not find too many loose objects", | |||
gc.tooManyLooseObjects()); | |||
} | |||
@Test | |||
public void testTooManyLooseObjects() throws Exception { | |||
FileBasedConfig c = repo.getConfig(); | |||
c.setInt(ConfigConstants.CONFIG_GC_SECTION, null, | |||
ConfigConstants.CONFIG_KEY_AUTO, 255); | |||
c.save(); | |||
commitChain(10, 50); | |||
assertTrue("should find too many loose objects", | |||
gc.tooManyLooseObjects()); | |||
} | |||
@Test | |||
public void testNotTooManyPacks() { | |||
assertFalse("should not find too many packs", gc.tooManyPacks()); | |||
} | |||
@Test | |||
public void testTooManyPacks() throws Exception { | |||
FileBasedConfig c = repo.getConfig(); | |||
c.setInt(ConfigConstants.CONFIG_GC_SECTION, null, | |||
ConfigConstants.CONFIG_KEY_AUTOPACKLIMIT, 1); | |||
c.save(); | |||
SampleDataRepositoryTestCase.copyCGitTestPacks(repo); | |||
assertTrue("should find too many packs", gc.tooManyPacks()); | |||
} | |||
} |
@@ -105,6 +105,46 @@ public abstract class GcTestCase extends LocalDiskRepositoryTestCase { | |||
return tip; | |||
} | |||
/** | |||
* Create a chain of commits of given depth with given number of added files | |||
* per commit. | |||
* <p> | |||
* Each commit contains {@code files} files as its content. The created | |||
* commit chain is referenced from any ref. | |||
* <p> | |||
* A chain will create {@code (2 + files) * depth} objects in Gits object | |||
* database. For each depth level the following objects are created: the | |||
* commit object, the top-level tree object and @code files} blobs for the | |||
* content of the file "a". | |||
* | |||
* @param depth | |||
* the depth of the commit chain. | |||
* @param width | |||
* number of files added per commit | |||
* @return the commit that is the tip of the commit chain | |||
* @throws Exception | |||
*/ | |||
protected RevCommit commitChain(int depth, int width) throws Exception { | |||
if (depth <= 0) { | |||
throw new IllegalArgumentException("Chain depth must be > 0"); | |||
} | |||
if (width <= 0) { | |||
throw new IllegalArgumentException("Number of files per commit must be > 0"); | |||
} | |||
CommitBuilder cb = tr.commit(); | |||
RevCommit tip = null; | |||
do { | |||
--depth; | |||
for (int i=0; i < width; i++) { | |||
String id = depth + "-" + i; | |||
cb.add("a" + id, id).message(id); | |||
} | |||
tip = cb.create(); | |||
cb = cb.child(); | |||
} while (depth > 0); | |||
return tip; | |||
} | |||
protected long lastModified(AnyObjectId objectId) throws IOException { | |||
return repo.getFS().lastModified( | |||
repo.getObjectDatabase().fileFor(objectId)); |
@@ -47,7 +47,9 @@ | |||
package org.eclipse.jgit.test.resources; | |||
import java.io.File; | |||
import java.io.IOException; | |||
import org.eclipse.jgit.internal.storage.file.FileRepository; | |||
import org.eclipse.jgit.junit.JGitTestUtil; | |||
import org.eclipse.jgit.junit.RepositoryTestCase; | |||
@@ -57,7 +59,17 @@ public abstract class SampleDataRepositoryTestCase extends RepositoryTestCase { | |||
@Override | |||
public void setUp() throws Exception { | |||
super.setUp(); | |||
copyCGitTestPacks(db); | |||
} | |||
/** | |||
* Copy C Git generated pack files into given repository for testing | |||
* | |||
* @param repo | |||
* test repository to receive packfile copies | |||
* @throws IOException | |||
*/ | |||
public static void copyCGitTestPacks(FileRepository repo) throws IOException { | |||
final String[] packs = { | |||
"pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f", | |||
"pack-df2982f284bbabb6bdb59ee3fcc6eb0983e20371", | |||
@@ -67,13 +79,14 @@ public abstract class SampleDataRepositoryTestCase extends RepositoryTestCase { | |||
"pack-e6d07037cbcf13376308a0a995d1fa48f8f76aaa", | |||
"pack-3280af9c07ee18a87705ef50b0cc4cd20266cf12" | |||
}; | |||
final File packDir = new File(db.getObjectDatabase().getDirectory(), "pack"); | |||
final File packDir = new File(repo.getObjectDatabase().getDirectory(), | |||
"pack"); | |||
for (String n : packs) { | |||
JGitTestUtil.copyTestResource(n + ".pack", new File(packDir, n + ".pack")); | |||
JGitTestUtil.copyTestResource(n + ".idx", new File(packDir, n + ".idx")); | |||
} | |||
JGitTestUtil.copyTestResource("packed-refs", | |||
new File(db.getDirectory(), "packed-refs")); | |||
new File(repo.getDirectory(), "packed-refs")); | |||
} | |||
} |
@@ -1164,7 +1164,7 @@ public class GC { | |||
/** | |||
* @return {@code true} if number of packs > gc.autopacklimit (default 50) | |||
*/ | |||
private boolean tooManyPacks() { | |||
boolean tooManyPacks() { | |||
int autopacklimit = repo.getConfig().getInt( | |||
ConfigConstants.CONFIG_GC_SECTION, | |||
ConfigConstants.CONFIG_KEY_AUTOPACKLIMIT, | |||
@@ -1183,7 +1183,7 @@ public class GC { | |||
* | |||
* @return {@code true} if number of loose objects > gc.auto (default 6700) | |||
*/ | |||
private boolean tooManyLooseObjects() { | |||
boolean tooManyLooseObjects() { | |||
int auto = repo.getConfig().getInt(ConfigConstants.CONFIG_GC_SECTION, | |||
ConfigConstants.CONFIG_KEY_AUTO, DEFAULT_AUTOLIMIT); | |||
if (auto <= 0) { | |||
@@ -1204,9 +1204,9 @@ public class GC { | |||
.matches(); | |||
} | |||
})) { | |||
Iterator<Path> iter = stream.iterator(); | |||
while (iter.hasNext()) { | |||
if (n++ > threshold) { | |||
for (Iterator<Path> iter = stream.iterator(); iter.hasNext(); | |||
iter.next()) { | |||
if (++n > threshold) { | |||
return true; | |||
} | |||
} |