PushCommand: allow users to disable use of bitmaps for push

Reachability bitmaps are designed to speed up the "counting objects"
phase of generating a pack during a clone or fetch. They are not
optimized for Git clients sending a small topic branch via "git push".
In some cases (see [1]), using reachability bitmaps during "git push"
can cause significant performance regressions.

Add PushCommand#setUseBitmaps(boolean) to allow users to tell "git push"
not to use bitmaps.

[1]: https://lore.kernel.org/git/87zhoz8b9o.fsf@evledraar.gmail.com/

Change-Id: I7fb7d26084ec63ddfa7249cf58abb85929b30e56
Signed-off-by: kylezhao <kylezhao@tencent.com>

5 files changed, 115 insertions, 1 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TransportTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TransportTest.java
index 5ae440f1d2..2019c263fb 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TransportTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TransportTest.java
@@ -242,6 +242,37 @@ public class TransportTest extends SampleDataRepositoryTestCase {
 	}
 
 	@Test
+	public void testOpenPushUseBitmaps() throws Exception {
+		URIish uri = new URIish("file://" + db.getWorkTree().getAbsolutePath());
+		// default
+		try (Transport transport = Transport.open(uri)) {
+			try (PushConnection pushConnection = transport.openPush()) {
+				assertTrue(pushConnection instanceof BasePackPushConnection);
+				BasePackPushConnection basePackPushConnection = (BasePackPushConnection) pushConnection;
+				assertEquals(true, basePackPushConnection.isUseBitmaps());
+			}
+		}
+		// true
+		try (Transport transport = Transport.open(uri)) {
+			transport.setPushUseBitmaps(true);
+			try (PushConnection pushConnection = transport.openPush()) {
+				assertTrue(pushConnection instanceof BasePackPushConnection);
+				BasePackPushConnection basePackPushConnection = (BasePackPushConnection) pushConnection;
+				assertEquals(true, basePackPushConnection.isUseBitmaps());
+			}
+		}
+		// false
+		try (Transport transport = Transport.open(uri)) {
+			transport.setPushUseBitmaps(false);
+			try (PushConnection pushConnection = transport.openPush()) {
+				assertTrue(pushConnection instanceof BasePackPushConnection);
+				BasePackPushConnection basePackPushConnection = (BasePackPushConnection) pushConnection;
+				assertEquals(false, basePackPushConnection.isUseBitmaps());
+			}
+		}
+	}
+
+	@Test
 	public void testSpi() {
 		List<TransportProtocol> protocols = Transport.getTransportProtocols();
 		assertNotNull(protocols);
diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
new file mode 100644
index 0000000000..6992fa9148
--- /dev/null
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<component id="org.eclipse.jgit" version="2">
+    <resource path="src/org/eclipse/jgit/transport/Transport.java" type="org.eclipse.jgit.transport.Transport">
+        <filter id="336658481">
+            <message_arguments>
+                <message_argument value="org.eclipse.jgit.transport.Transport"/>
+                <message_argument value="DEFAULT_PUSH_USE_BITMAPS"/>
+            </message_arguments>
+        </filter>
+    </resource>
+</component>
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java
index c4fb7a2184..2ed1c52fd7 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java
@@ -74,6 +74,7 @@ public class PushCommand extends
 	private boolean atomic;
 	private boolean force;
 	private boolean thin = Transport.DEFAULT_PUSH_THIN;
+	private boolean useBitmaps = Transport.DEFAULT_PUSH_USE_BITMAPS;
 
 	private PrintStream hookOutRedirect;
 
@@ -145,6 +146,7 @@ public class PushCommand extends
 					transport.setOptionReceivePack(receivePack);
 				transport.setDryRun(dryRun);
 				transport.setPushOptions(pushOptions);
+				transport.setPushUseBitmaps(useBitmaps);
 				transport.setHookOutputStream(hookOutRedirect);
 				transport.setHookErrorStream(hookErrRedirect);
 				configure(transport);
@@ -623,6 +625,32 @@ public class PushCommand extends
 	}
 
 	/**
+	 * Whether to use bitmaps for push.
+	 *
+	 * @return true if push use bitmaps.
+	 * @since 6.4
+	 */
+	public boolean isUseBitmaps() {
+		return useBitmaps;
+	}
+
+	/**
+	 * Set whether to use bitmaps for push.
+	 *
+	 * Default setting is {@value Transport#DEFAULT_PUSH_USE_BITMAPS}
+	 *
+	 * @param useBitmaps
+	 *            false to disable use of bitmaps for push, true otherwise.
+	 * @return {@code this}
+	 * @since 6.4
+	 */
+	public PushCommand setUseBitmaps(boolean useBitmaps) {
+		checkCallable();
+		this.useBitmaps = useBitmaps;
+		return this;
+	}
+
+	/**
 	 * Whether this push should be executed atomically (all references updated,
 	 * or none)
 	 *
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
index b7be59d6f8..adc1c9849d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
@@ -90,6 +90,7 @@ public abstract class BasePackPushConnection extends BasePackConnection implemen
 
 	private final boolean thinPack;
 	private final boolean atomic;
+	private final boolean useBitmaps;
 
 	/** A list of option strings associated with this push. */
 	private List<String> pushOptions;
@@ -118,6 +119,7 @@ public abstract class BasePackPushConnection extends BasePackConnection implemen
 		thinPack = transport.isPushThin();
 		atomic = transport.isPushAtomic();
 		pushOptions = transport.getPushOptions();
+		useBitmaps = transport.isPushUseBitmaps();
 	}
 
 	/** {@inheritDoc} */
@@ -320,7 +322,7 @@ public abstract class BasePackPushConnection extends BasePackConnection implemen
 
 			writer.setIndexDisabled(true);
 			writer.setUseCachedPacks(true);
-			writer.setUseBitmaps(true);
+			writer.setUseBitmaps(useBitmaps);
 			writer.setThin(thinPack);
 			writer.setReuseValidatingObjects(false);
 			writer.setDeltaBaseAsOffset(capableOfsDelta);
@@ -421,6 +423,16 @@ public abstract class BasePackPushConnection extends BasePackConnection implemen
 		return pushOptions;
 	}
 
+	/**
+	 * Whether to use bitmaps for push.
+	 *
+	 * @return true if push use bitmaps.
+	 * @since 6.4
+	 */
+	public boolean isUseBitmaps() {
+		return useBitmaps;
+	}
+
 	private static class CheckingSideBandOutputStream extends OutputStream {
 		private final InputStream in;
 		private final OutputStream out;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java
index c3271ebc76..ee35f4866e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java
@@ -705,6 +705,13 @@ public abstract class Transport implements AutoCloseable {
 	public static final boolean DEFAULT_PUSH_THIN = false;
 
 	/**
+	 * Default setting for {@link #pushUseBitmaps} option.
+	 *
+	 * @since 6.4
+	 */
+	public static final boolean DEFAULT_PUSH_USE_BITMAPS = true;
+
+	/**
 	 * Specification for fetch or push operations, to fetch or push all tags.
 	 * Acts as --tags.
 	 */
@@ -756,6 +763,9 @@ public abstract class Transport implements AutoCloseable {
 	/** Should push be all-or-nothing atomic behavior? */
 	private boolean pushAtomic;
 
+	/** Should push use bitmaps? */
+	private boolean pushUseBitmaps = DEFAULT_PUSH_USE_BITMAPS;
+
 	/** Should push just check for operation result, not really push. */
 	private boolean dryRun;
 
@@ -1053,6 +1063,28 @@ public abstract class Transport implements AutoCloseable {
 	}
 
 	/**
+	 * Default setting is: {@value #DEFAULT_PUSH_USE_BITMAPS}
+	 *
+	 * @return true if push use bitmaps.
+	 * @since 6.4
+	 */
+	public boolean isPushUseBitmaps() {
+		return pushUseBitmaps;
+	}
+
+	/**
+	 * Set whether to use bitmaps for push. Default setting is:
+	 * {@value #DEFAULT_PUSH_USE_BITMAPS}
+	 *
+	 * @param useBitmaps
+	 *            false to disable use of bitmaps for push, true otherwise.
+	 * @since 6.4
+	 */
+	public void setPushUseBitmaps(boolean useBitmaps) {
+		this.pushUseBitmaps = useBitmaps;
+	}
+
+	/**
 	 * Whether destination refs should be removed if they no longer exist at the
 	 * source repository.
 	 *