From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-0.6 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by dcvr.yhbt.net (Postfix) with ESMTP id 1194C1F729 for ; Thu, 16 Jun 2022 18:12:44 +0000 (UTC) Authentication-Results: dcvr.yhbt.net; dkim=pass (1024-bit key; unprotected) header.d=pobox.com header.i=@pobox.com header.b="epxQHC0A"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377330AbiFPSMi (ORCPT ); Thu, 16 Jun 2022 14:12:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46556 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233838AbiFPSMg (ORCPT ); Thu, 16 Jun 2022 14:12:36 -0400 Received: from pb-smtp20.pobox.com (pb-smtp20.pobox.com [173.228.157.52]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4B54626F6 for ; Thu, 16 Jun 2022 11:12:34 -0700 (PDT) Received: from pb-smtp20.pobox.com (unknown [127.0.0.1]) by pb-smtp20.pobox.com (Postfix) with ESMTP id AEE791AF338; Thu, 16 Jun 2022 14:12:33 -0400 (EDT) (envelope-from junio@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type:content-transfer-encoding; s=sasl; bh=Qy+IayMP0div LAdq5GhI4RbiO4oJjeRkx7ExKOyp5/I=; b=epxQHC0AwGKO3D94kwC7BBLIZeNm KhlxEbLySjwYN9uQwdFrEsEV54nss+HnG3HHXnXpQzd1R0GGOqzHlIkgalkRzJfN njuPEX08ee5Ak/FMoRJ0ks7Oi92/HrVqdbfHcCWHlruYe5EEIpmd1C3QdZYO3pdW 0dd8BntMNk7/pFg= Received: from pb-smtp20.sea.icgroup.com (unknown [127.0.0.1]) by pb-smtp20.pobox.com (Postfix) with ESMTP id A70741AF337; Thu, 16 Jun 2022 14:12:33 -0400 (EDT) (envelope-from junio@pobox.com) Received: from pobox.com (unknown [34.82.80.254]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp20.pobox.com (Postfix) with ESMTPSA id 3FB5C1AF336; Thu, 16 Jun 2022 14:12:30 -0400 (EDT) (envelope-from junio@pobox.com) From: Junio C Hamano To: =?utf-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason Cc: Kyle Zhao via GitGitGadget , git@vger.kernel.org, Derrick Stolee , Taylor Blau , Kyle Zhao Subject: Re: [PATCH v2] send-pack.c: add config push.useBitmaps References: <220616.86fsk4ww69.gmgdl@evledraar.gmail.com> Date: Thu, 16 Jun 2022 11:12:28 -0700 In-Reply-To: <220616.86fsk4ww69.gmgdl@evledraar.gmail.com> (=?utf-8?B?IsOG?= =?utf-8?B?dmFyIEFybmZqw7Zyw7A=?= Bjarmason"'s message of "Thu, 16 Jun 2022 15:38:36 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 X-Pobox-Relay-ID: E2D429C6-ED9F-11EC-BEA8-C85A9F429DF0-77302942!pb-smtp20.pobox.com Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org =C3=86var Arnfj=C3=B6r=C3=B0 Bjarmason writes: >> + mk_test testrepo heads/main && >> + git checkout main && >> + GIT_TRACE2_EVENT=3D"$PWD/default" \ >> + git push testrepo main:test && >> + test_subcommand git pack-objects --all-progress-implied --revs --std= out \ >> + --thin --delta-base-offset -q --no-use-bitmap-index > Nit: We tend to indent these ase we wrap, so e.g.: > > test_subcommand git ... \ > --thin --delta [...] > > The rest all looks good as far as the diff goes, if what we want to do > is to disable this by default, and this isn't worth a re-roll in itself= . > > But I still think that completely disabling bitmaps might be premature > here, especially per Taylor's comment on v1 (which I understand to mean > that they should help some of the time, even with push). The usual way to move is to move slowly and carefully. It may well be the case that disabling bitmaps gives users a better default, but that is not even proven and is hard to prove. How many users of Git do we have? Those silently using it happily will by definition complain here or elsewhere, and the complaints "X is slow with Y, so Y should be disabled when doing X" we hear tend to be louder than "I am happily doing X with Y". I have different problems with this patch, though. It can use a bit more honesty. If you introduce a new knob and sell it as a knob that allows disabling, be honest and keep its behaviour as advertised. As posted, IIUC, the patch does something quite different. It disables by default, and have a knob to allow it enabled again. So, perhaps make it default on to keep the historical behaviour, and document it as "setting it false may improve push performance without affecting use of the reachability bitmaps for other operations. Default is true." Thanks.