From ad9c217f4954dc50f629b3eb6ae387b6940a5023 Mon Sep 17 00:00:00 2001 From: kylezhao Date: Thu, 14 Jul 2022 11:23:03 +0800 Subject: [PATCH] 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 --- .../eclipse/jgit/transport/TransportTest.java | 31 ++++++++++++++++++ org.eclipse.jgit/.settings/.api_filters | 11 +++++++ .../src/org/eclipse/jgit/api/PushCommand.java | 28 ++++++++++++++++ .../transport/BasePackPushConnection.java | 14 +++++++- .../org/eclipse/jgit/transport/Transport.java | 32 +++++++++++++++++++ 5 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 org.eclipse.jgit/.settings/.api_filters 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 @@ -241,6 +241,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 protocols = Transport.getTransportProtocols(); 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 @@ + + + + + + + + + + + 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); @@ -622,6 +624,32 @@ public class PushCommand extends 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) 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 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 @@ -704,6 +704,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; @@ -1052,6 +1062,28 @@ public abstract class Transport implements AutoCloseable { 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. -- 2.39.5