UploadPack: Clear advertised ref map after negotiation
After negotiation phase of a fetch, the advertised ref map is no longer used and
can be safely cleared. For >1GiB repos object selection and packfile writing may
take 10s of minutes. For the chromium.googlesource.com/chromium/src repo, this
advertised ref map is >400MiB. Returning this memory to the Java heap is a major
scalability win.
Change-Id: I00d453c5ef47630c21f199e333e1cfcf47b7e92a
Signed-off-by: Minh Thai <mthai@google.com>
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
index ea86563..d58e576 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
@@ -44,6 +44,7 @@
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.ProgressMonitor;
import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.Sets;
import org.eclipse.jgit.lib.TextProgressMonitor;
@@ -2238,4 +2239,81 @@
}
}
+ @Test
+ public void testSafeToClearRefsInFetchV0() throws Exception {
+ server =
+ new RefCallsCountingRepository(
+ new DfsRepositoryDescription("server"));
+ remote = new TestRepository<>(server);
+ RevCommit one = remote.commit().message("1").create();
+ remote.update("one", one);
+ testProtocol = new TestProtocol<>((Object req, Repository db) -> {
+ UploadPack up = new UploadPack(db);
+ return up;
+ }, null);
+ uri = testProtocol.register(ctx, server);
+ try (Transport tn = testProtocol.open(uri, client, "server")) {
+ tn.fetch(NullProgressMonitor.INSTANCE,
+ Collections.singletonList(new RefSpec(one.name())));
+ }
+ assertTrue(client.getObjectDatabase().has(one.toObjectId()));
+ assertEquals(1, ((RefCallsCountingRepository)server).numRefCalls());
+ }
+
+ @Test
+ public void testSafeToClearRefsInFetchV2() throws Exception {
+ server =
+ new RefCallsCountingRepository(
+ new DfsRepositoryDescription("server"));
+ remote = new TestRepository<>(server);
+ RevCommit one = remote.commit().message("1").create();
+ RevCommit two = remote.commit().message("2").create();
+ remote.update("one", one);
+ remote.update("two", two);
+ server.getConfig().setBoolean("uploadpack", null, "allowrefinwant", true);
+ ByteArrayInputStream recvStream = uploadPackV2(
+ "command=fetch\n",
+ PacketLineIn.delimiter(),
+ "want-ref refs/heads/one\n",
+ "want-ref refs/heads/two\n",
+ "done\n",
+ PacketLineIn.end());
+ PacketLineIn pckIn = new PacketLineIn(recvStream);
+ assertThat(pckIn.readString(), is("wanted-refs"));
+ assertThat(
+ Arrays.asList(pckIn.readString(), pckIn.readString()),
+ hasItems(
+ one.toObjectId().getName() + " refs/heads/one",
+ two.toObjectId().getName() + " refs/heads/two"));
+ assertTrue(PacketLineIn.isDelimiter(pckIn.readString()));
+ assertThat(pckIn.readString(), is("packfile"));
+ parsePack(recvStream);
+ assertTrue(client.getObjectDatabase().has(one.toObjectId()));
+ assertEquals(1, ((RefCallsCountingRepository)server).numRefCalls());
+ }
+
+ private class RefCallsCountingRepository extends InMemoryRepository {
+ private final InMemoryRepository.MemRefDatabase refdb;
+ private int numRefCalls;
+
+ public RefCallsCountingRepository(DfsRepositoryDescription repoDesc) {
+ super(repoDesc);
+ refdb = new InMemoryRepository.MemRefDatabase() {
+ @Override
+ public List<Ref> getRefs() throws IOException {
+ numRefCalls++;
+ return super.getRefs();
+ }
+ };
+ }
+
+ public int numRefCalls() {
+ return numRefCalls;
+ }
+
+ @Override
+ public RefDatabase getRefDatabase() {
+ return refdb;
+ }
+ }
}
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 35196c6..bb826d8 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -298,7 +298,7 @@
private boolean sentReady;
- /** Objects we sent in our advertisement list, clients can ask for these. */
+ /** Objects we sent in our advertisement list. */
private Set<ObjectId> advertised;
/** Marked on objects the client has asked us to give them. */
@@ -381,8 +381,10 @@
/**
* Get refs which were advertised to the client.
*
- * @return all refs which were advertised to the client, or null if
- * {@link #setAdvertisedRefs(Map)} has not been called yet.
+ * @return all refs which were advertised to the client. Only valid during
+ * the negotiation phase. Will return {@code null} if
+ * {@link #setAdvertisedRefs(Map)} has not been called yet or if
+ * {@code #sendPack()} has been called.
*/
public final Map<String, Ref> getAdvertisedRefs() {
return refs;
@@ -2205,6 +2207,11 @@
}
msgOut.flush();
+ // Advertised objects and refs are not used from here on and can be
+ // cleared.
+ advertised = null;
+ refs = null;
+
PackConfig cfg = packConfig;
if (cfg == null)
cfg = new PackConfig(db);
