[PATCH v2] send-pack.c: add config push.useBitmaps

From: Kyle Zhao <kylezhao@tencent.com>

This allows you to disable bitmaps for "git push". Default is false.

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 a new "push.useBitmaps" config option to disable reachability bitmaps during "git push". This allows reachability bitmaps to still be used in other areas, such as "git rev-list --use-bitmap-index".

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

Signed-off-by: Kyle Zhao <kylezhao@tencent.com>
---
    send-pack.c: add config push.useBitmaps
    
    This patch add config push.useBitmaps to prevent git push using bitmap.
    
    The origin discussion is here:
    https://lore.kernel.org/git/b940e705fbe9454685757f2e3055e2ce@tencent.com/
    
    Thanks, -Kyle
    
    Changes since v1:
    
     * changed the commit message
     * modified and added missing \n to push.txt
     * used test_subcommand for test
     * modified "if" statement for "git_config_get_bool()" in send-pack.c

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1263%2Fkeyu98%2Fkz%2Fpush-usebitmps-config-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1263/keyu98/kz/push-usebitmps-config-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1263

Range-diff vs v1:

 1:  000d033584b ! 1:  42e0b4845b2 send-pack.c: add config push.useBitmaps
     @@ Metadata
       ## Commit message ##
          send-pack.c: add config push.useBitmaps
      
     -    This allows you to disabled bitmaps for "git push". Default is false.
     +    This allows you to disable bitmaps for "git push". Default is false.
      
     -    Bitmaps are designed to speed up the "counting objects" phase of
     -    subsequent packs created for clones and fetches.
     -    But in some cases, turning bitmaps on does horrible things for "push"
     -    performance[1].
     +    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 a new "push.useBitmaps" config option to disable reachability
     +    bitmaps during "git push". This allows reachability bitmaps to still
     +    be used in other areas, such as "git rev-list --use-bitmap-index".
      
          [1]: https://lore.kernel.org/git/87zhoz8b9o.fsf@evledraar.gmail.com/
      
     @@ Documentation/config/push.txt: push.negotiate::
       	in common.
      +
      +push.useBitmaps::
     -+	If this config and `pack.useBitmaps` are both "true", git will
     -+	use pack bitmaps (if available) when git push. Default is false.
     - \ No newline at end of file
     ++    If this config and `pack.useBitmaps` are both `true`, then Git will
     ++    use reachability bitmaps during `git push`, if available (disabled
     ++    by default).
      
       ## send-pack.c ##
      @@ send-pack.c: static int pack_objects(int fd, struct ref *refs, struct oid_array *advertised,
     @@ send-pack.c: static int pack_objects(int fd, struct ref *refs, struct oid_array
       	po.out = args->stateless_rpc ? -1 : fd;
       	po.git_cmd = 1;
      @@ send-pack.c: int send_pack(struct send_pack_args *args,
     - 	int use_push_options = 0;
     - 	int push_options_supported = 0;
     - 	int object_format_supported = 0;
     -+	int use_bitmaps = 0;
     - 	unsigned cmds_sent = 0;
     - 	int ret;
       	struct async demux;
     + 	const char *push_cert_nonce = NULL;
     + 	struct packet_reader reader;
     ++	int use_bitmaps;
     + 
     + 	if (!remote_refs) {
     + 		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
      @@ send-pack.c: int send_pack(struct send_pack_args *args,
     - 	git_config_get_bool("push.negotiate", &push_negotiate);
       	if (push_negotiate)
       		get_commons_through_negotiation(args->url, remote_refs, &commons);
     -+	git_config_get_bool("push.usebitmaps", &use_bitmaps);
     -+	if (use_bitmaps)
     -+		args->use_bitmaps = 1;
       
     ++	if (!git_config_get_bool("push.usebitmaps", &use_bitmaps))
     ++		args->use_bitmaps = use_bitmaps;
     ++
       	git_config_get_bool("transfer.advertisesid", &advertise_sid);
       
     + 	/* Does the other end support the reporting? */
      
       ## send-pack.h ##
      @@ send-pack.h: struct send_pack_args {
     @@ t/t5516-fetch-push.sh: test_expect_success 'push warns or fails when using usern
      +test_expect_success 'push with config push.useBitmaps' '
      +	mk_test testrepo heads/main &&
      +	git checkout main &&
     -+	GIT_TRACE=1 git push testrepo main:test >/dev/null 2>stderr &&
     -+	grep "no-use-bitmap-index" stderr &&
     ++	GIT_TRACE2_EVENT="$PWD/default" \
     ++	git push testrepo main:test &&
     ++	test_subcommand git pack-objects --all-progress-implied --revs --stdout \
     ++	--thin --delta-base-offset -q --no-use-bitmap-index <default &&
      +
      +	git config push.useBitmaps true &&
     -+	GIT_TRACE=1 git push testrepo main:test2 >/dev/null 2>stderr &&
     -+	! grep "no-use-bitmap-index" stderr
     ++	GIT_TRACE2_EVENT="$PWD/true" \
     ++	git push testrepo main:test2 &&
     ++	test_subcommand git pack-objects --all-progress-implied --revs --stdout \
     ++	--thin --delta-base-offset -q <true &&
     ++
     ++	git config push.useBitmaps false &&
     ++	GIT_TRACE2_EVENT="$PWD/false" \
     ++	git push testrepo main:test3 &&
     ++	test_subcommand git pack-objects --all-progress-implied --revs --stdout \
     ++	--thin --delta-base-offset -q --no-use-bitmap-index <false
      +'
      +
       test_done

 Documentation/config/push.txt |  5 +++++
 send-pack.c                   |  6 ++++++
 send-pack.h                   |  3 ++-
 t/t5516-fetch-push.sh         | 21 +++++++++++++++++++++
 4 files changed, 34 insertions(+), 1 deletion(-)