[PATCH v3 0/4] Restrict when prefetcing occurs

From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>,
	garimasigit@gmail.com, gitster@pobox.com
Subject: [PATCH v3 0/4] Restrict when prefetcing occurs
Date: Tue,  7 Apr 2020 15:11:39 -0700	[thread overview]
Message-ID: <cover.1586296510.git.jonathantanmy@google.com> (raw)
In-Reply-To: <20200331020418.55640-1-jonathantanmy@google.com>

Junio wrote in [1]:

> s/rebase/rename/ I presume, but the above reasoning, while it may
> happen to hold true right now, feels brittle.  In other words
> 
>  - how do we know it would stay to be "a superset"?
> 
>  - would it change the picture if we later added a prefetch in
>    diffcore_break(), just like you are doing so to diffcore_rename()
>    in this patch?

and suggested that each function be capable of prefetching. I've done
that for most functions in this new version.

To avoid the potential slowdown of each function doing its own object
existence checks (that is, looping through all the relevant OIDs and
then prefetching based on whether it found one missing), what I did is
to teach diff_populate_filespec() to retry whenever it attempts to read
a missing object, calling a callback before its 2nd (and final) try. I
then taught the functions called by diffcore_std() to pass a prefetching
function as this callback.

The functions I've taught include diffcore_skip_stat_unmatch(). I
couldn't figure out how to trigger this behavior in a test (I can see
that the function is being run, but not how to make it read an object),
but I included the prefetching mechanism in this function anyway for
completeness.

The previous version of my patch [2] made the assumption that the
fetching done at the start of diffcore_std() is a superset of the
fetching done by diffcore_rebase() - hence Junio's comment above about
how we would know that it would stay a superset. With this series, if
ever that no longer holds (and we miss fixing it), rebase would only do
one additional bulk fetch (instead of fetching once for every missing
object).

[1] https://lore.kernel.org/git/xmqqsghl1m0p.fsf@gitster.c.googlers.com/
[2] https://lore.kernel.org/git/a3322cdedf019126305fcead5918d523a1b2dfbc.1585854639.git.jonathantanmy@google.com/

Jonathan Tan (4):
  promisor-remote: accept 0 as oid_nr in function
  diff: make diff_populate_filespec_options struct
  diff: refactor object read
  diff: restrict when prefetching occurs

 builtin/index-pack.c          |   5 +-
 diff.c                        | 157 +++++++++++++++++++++++-----------
 diffcore-break.c              |  12 ++-
 diffcore-rename.c             |  64 ++++++++++++--
 diffcore.h                    |  30 ++++++-
 line-log.c                    |   6 +-
 promisor-remote.c             |   3 +
 promisor-remote.h             |   8 ++
 t/t4067-diff-partial-clone.sh |  48 +++++++++++
 unpack-trees.c                |   5 +-
 10 files changed, 267 insertions(+), 71 deletions(-)

-- 
2.26.0.292.g33ef6b2f38-goog