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>
This commit is contained in:
kylezhao 2022-07-14 11:23:03 +08:00 committed by Thomas Wolf
parent 71af0d6a5c
commit ad9c217f49
5 changed files with 115 additions and 1 deletions

View File

@ -241,6 +241,37 @@ public void testLocalTransportFetchWithoutLocalRepository()
}
}
@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();

View File

@ -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>

View File

@ -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 Iterable<PushResult> call() throws GitAPIException,
transport.setOptionReceivePack(receivePack);
transport.setDryRun(dryRun);
transport.setPushOptions(pushOptions);
transport.setPushUseBitmaps(useBitmaps);
transport.setHookOutputStream(hookOutRedirect);
transport.setHookErrorStream(hookErrRedirect);
configure(transport);
@ -622,6 +624,32 @@ public PushCommand setThin(boolean thin) {
return this;
}
/**
* 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)

View File

@ -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 BasePackPushConnection(PackTransport packTransport) {
thinPack = transport.isPushThin();
atomic = transport.isPushAtomic();
pushOptions = transport.getPushOptions();
useBitmaps = transport.isPushUseBitmaps();
}
/** {@inheritDoc} */
@ -320,7 +322,7 @@ private void writePack(final Map<String, RemoteRefUpdate> refUpdates,
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 List<String> getPushOptions() {
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;

View File

@ -704,6 +704,13 @@ static String findTrackingRefName(final String remoteName,
*/
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 @@ static String findTrackingRefName(final String remoteName,
/** 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;
@ -1052,6 +1062,28 @@ public void setPushAtomic(boolean atomic) {
this.pushAtomic = atomic;
}
/**
* 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.