[0/4] Don't lazy-fetch commits when parsing them

Message

Jonathan Tan Nov. 30, 2022, 8:30 p.m. UTC

This is a follow-up from my previous email about the possibility of not
fetching when we know that we're fetching a commit [1]. I had to refactor a few
things mostly due to replace objects, so the number of patches might be larger
than you would expect. I tried to keep each patch small and easy to understand,
though.

Patches 1-3 contain some forward-compatibility and refactoring changes, and
patch 4 contains the actual logic change.

[1] https://lore.kernel.org/git/[e-mail]/

Jonathan Tan (4):
  object-file: reread object with exact same args
  object-file: refactor corrupt object diagnosis
  object-file: refactor replace object lookup
  commit: don't lazy-fetch commits

 commit.c       | 18 ++++++++++++++--
 object-file.c  | 57 ++++++++++++++++++++++++++++++++++----------------
 object-store.h | 10 +++++++++
 3 files changed, 65 insertions(+), 20 deletions(-)

Comments

Jeff King Nov. 30, 2022, 9:06 p.m. UTC | #1

On Wed, Nov 30, 2022 at 12:30:45PM -0800, Jonathan Tan wrote:

> This is a follow-up from my previous email about the possibility of not
> fetching when we know that we're fetching a commit [1]. I had to refactor a few
> things mostly due to replace objects, so the number of patches might be larger
> than you would expect. I tried to keep each patch small and easy to understand,
> though.
> 
> Patches 1-3 contain some forward-compatibility and refactoring changes, and
> patch 4 contains the actual logic change.

These look pretty good to me. I raised a minor nit in patch 2; if you
agree it should be a trivial re-roll.

I left some thoughts on the approach in patch 4, but I think given that
this is a strict improvement over the status quo, it's a good step
forward, even if it won't catch all such cases.

-Peff