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,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 CCC7B1F4CE for ; Fri, 1 Apr 2022 10:53:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345233AbiDAKyI (ORCPT ); Fri, 1 Apr 2022 06:54:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37750 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345440AbiDAKx4 (ORCPT ); Fri, 1 Apr 2022 06:53:56 -0400 Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C4D0960DA7 for ; Fri, 1 Apr 2022 03:52:07 -0700 (PDT) Received: by mail-pj1-x102a.google.com with SMTP id gb19so2103205pjb.1 for ; Fri, 01 Apr 2022 03:52:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=coup.net.nz; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=jC52LNWQ8AVPKNfpPAPkH/GzTpjjTdrh++5uLd18IuA=; b=fEWm7Z4QmAWW6qiyENU7K7tGvFQNnU9N4uZduK1ygU2+qVsL8+5qADIt0eiKbEAV7Y 2coYQ6y1ltlK+CGbN0ev3fRWoBV+gY5Jo79TbZ29xmxLqkN4DjHIAf6jTEFdJCkZUCqM 8Ol+nPVYTqCXy1bCVZgehu52ObgJsr59YZBSc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=jC52LNWQ8AVPKNfpPAPkH/GzTpjjTdrh++5uLd18IuA=; b=WFiB4ewFQZhvF8bRXjEXKBtwvMXWDvNH9uuqm/iRVTpM9+MtIo+OOiuq8U+M4HF8T9 fjOroYGQydoSs7ZpmETmKpXYma1QOT9VYXN/53/T3nTCBUWaQ3JP0t/5SoEfUsDGM/f9 mxldA5hrCF1ti5qlUQqT/jEjs0WlX/+5qAdmVkwg+/hizp/No6qZoE17cg2NMfcWPxdB 0EN6tq7mxxyQQKbn6RHjOw/m8Xb3DJ8xHuYYohHmvLBxxTq4543x8YGPJZYtXy28Ku1z F8oH5e9nPOvcYF/rETRn9VpFuPKDBI8vwgz/e0p9gQ6CtlPrEXo3hKaFpEPT60F58IPo DMoA== X-Gm-Message-State: AOAM533uL3jiz61g7V/cFsF0jnSAnXdhNANvl9Ef56LUVP1sFjZ8shOD gP11spSGp1xQ5qI6CKMQvqkZAXOpLSPUOwMwwemiEIuJ/cki3f2zwZ4= X-Google-Smtp-Source: ABdhPJz9I4N52EATV4kzjTP6voqmuKVaJopPoj4V/Iklpq2WkamQAYB+322h3xrWwvC00F/2G3rI9Og5vg/lQsE7SLM= X-Received: by 2002:a17:90b:4c09:b0:1c6:f64a:dd0 with SMTP id na9-20020a17090b4c0900b001c6f64a0dd0mr11187346pjb.45.1648810327229; Fri, 01 Apr 2022 03:52:07 -0700 (PDT) MIME-Version: 1.0 References: <28c07219fd830196af1171320b86bc2a58ba3d79.1648476132.git.gitgitgadget@gmail.com> <220331.86bkxmp25k.gmgdl@evledraar.gmail.com> In-Reply-To: <220331.86bkxmp25k.gmgdl@evledraar.gmail.com> From: Robert Coup Date: Fri, 1 Apr 2022 11:51:56 +0100 Message-ID: Subject: Re: [PATCH v4 6/7] fetch: after refetch, encourage auto gc repacking To: =?UTF-8?B?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Cc: Robert Coup via GitGitGadget , git@vger.kernel.org, Jonathan Tan , John Cai , Jeff Hostetler , Junio C Hamano , Derrick Stolee , Calvin Wan Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Hi =C3=86var, On Thu, 31 Mar 2022 at 16:33, =C3=86var Arnfj=C3=B6r=C3=B0 Bjarmason wrote: > > + if (git_config_get_int("gc.autopacklimit", &opt_v= al)) > > + opt_val =3D -1; > > + if (opt_val !=3D 0) > > nit: don't compare against 0 or null, just !opt_val I did this since 0 has a specific meaning ("Setting this to 0 disables"), it's not just false-y in this context. Tomayto, tomahto? > > Isn't this whole thing also clearer as: > > int &forget; > > if (git_conf...(..., &forget)) > git_config_push_parameter("gc.autoPackLimit=3D1"); > > Maybe I haven't eyeballed this enough, but aren't you ignoring explicit > gc.autoPackLimit=3D0 configuration? Whereas what you seem to want is "set > this config unlress the user has it set", for which we only need to > check the git_config...(...) return value, no? What I'm trying to achieve: if the user has not disabled auto-packing (autoPackLimit=3D0), then pass autoPackLimit=3D1 to the subprocess to encourage repacking. Context/why: so we don't 2x the object store size and not even attempt to repack it now, rather than at some unspecified point in the future. Maybe. How the code achieves it: load autoPackLimit into opt_val if autoPackLimit is not specified in config: set opt_val to -1 if opt_val is not 0: pass autoPackLimit=3D1 to the subprocess AFAICT if we just if(git_config_get_int()) then if they haven't set it at all in config, we wouldn't encourage repacking in the subprocess. Which isn't what I'm trying to achieve. > hrm, do we really need to set both of these these days (not saying we > don't, just surprised). I.e. both gc.* an maintenance.* config. > > *skims the code* > > Urgh, yes? too_many_packs() seems to check gc.* only, but > incremental_repack_auto_condition() check this variable... :( Yes. > > > +test_expect_success 'fetch --refetch triggers repacking' ' > > + GIT_TRACE2_CONFIG_PARAMS=3Dgc.autoPackLimit,maintenance.increment= al-repack.auto && > > Nit: Can we use GIT_CONFIG_KEY_* et al for this these days, or do we > still need this trace2 thingy? I copied a pattern existing tests are using. Thanks, Rob.