From: Jonathan Tan <jonathantanmy@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jonathan Tan <jonathantanmy@google.com>,
Kousik Sanagavarapu <five231003@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH v2] index-pack: remove fetch_if_missing=0
Date: Fri, 10 Mar 2023 18:59:06 -0800 [thread overview]
Message-ID: <20230311025906.4170554-1-jonathantanmy@google.com> (raw)
In-Reply-To: <xmqqttys4746.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Do we have other uses of fetch_if_missing (i.e. disable lazy
> fetching)?
>
> $ git grep -l fetch_if_missing
> Documentation/technical/partial-clone.txt
> builtin/fetch-pack.c
> builtin/fsck.c
> builtin/pack-objects.c
> builtin/prune.c
> builtin/rev-list.c
> cache.h
> midx.c
> object-file.c
> revision.c
>
> As the default is 1, all these hits (outside the header, doc, and
> object-file.c) are to disable lazy fetching. Judging from the list
> of "family" that want tighter control over what gets fetched, I have
> a feeling that pack-index may want to stay to be in the family.
I think this "family" concept is a good way to think of it. I did
use to think that it would be better to be consistent throughout Git
and choose one world-view, and if I had to choose, it would be the one
without fetch_if_missing=0. But now it does make sense to me to have
two families:
(a) The more low-level code that the lazy fetching itself relies on
(and maybe things like builtin/fsck.c as well) where we really need
to be careful about what we fetch, and it would be better to err
on the side of not fetching. The test cases for these would need to
cover both the partial clone cases and the regular cases.
For these cases, the consequence of lazy-fetching when we shouldn't
might be as bad as an infinite loop, so it makes sense to default
not lazy-fetching here.
(b) The more high-level code, in which I think that it is better to err
on the side of fetching. The test cases would generally not need to
cover the partial clone cases (except when there are specific
optimizations needed, such as in checkout where we bulk prefetch
missing objects).
For these cases, the consequences of lazy-fetching when we shouldn't
are generally performance-related, so it might not be so bad to let
development happen in these areas of code without great
consideration to whether a lazy-fetch would happen if an object
didn't exist. (I do think it would be ideal for all new code to pay
attention to when they read objects, which would help not only in
partial clone but also in a potential future in which we have non-
disk object stores, but we're probably not there yet as a project.)
And indeed, pack-index would go in (a).
next prev parent reply other threads:[~2023-03-11 2:59 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-25 5:24 [PATCH] index-pack: remove fetch_if_missing=0 Kousik Sanagavarapu
2023-02-27 16:56 ` Kousik Sanagavarapu
2023-02-27 22:14 ` Jonathan Tan
2023-02-28 3:54 ` Kousik Sanagavarapu
2023-03-10 18:30 ` [PATCH v2] " Kousik Sanagavarapu
2023-03-10 20:30 ` Junio C Hamano
2023-03-10 21:13 ` Jonathan Tan
2023-03-10 21:41 ` Junio C Hamano
2023-03-11 2:59 ` Jonathan Tan [this message]
2023-03-12 17:16 ` Kousik Sanagavarapu
2023-03-11 6:22 ` [PATCH] " Kousik Sanagavarapu
2023-03-11 6:00 ` Kousik Sanagavarapu
2023-03-13 18:15 ` [PATCH v3] " Kousik Sanagavarapu
2023-03-13 19:17 ` Junio C Hamano
2023-03-13 19:18 ` Junio C Hamano
2023-03-17 17:56 ` [PATCH v4] " Kousik Sanagavarapu
2023-03-17 22:58 ` Junio C Hamano
2023-03-19 6:17 ` Kousik Sanagavarapu
2023-03-11 20:01 ` [PATCH] " Sean Allred
2023-03-11 20:37 ` Junio C Hamano
[not found] <--in-reply-to=20230225052439.27096-1-five231003@gmail.com>
2023-03-10 18:20 ` [PATCH v2] " Kousik Sanagavarapu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230311025906.4170554-1-jonathantanmy@google.com \
--to=jonathantanmy@google.com \
--cc=five231003@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).