Re: [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern

From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Denton Liu <liu.denton@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern
Date: Thu, 8 Oct 2020 15:53:09 -0400	[thread overview]
Message-ID: <20201008195309.GA2841047@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqwo00woz5.fsf@gitster.c.googlers.com>

On Thu, Oct 08, 2020 at 11:29:34AM -0700, Junio C Hamano wrote:

> >   Side note: It's a shame that there is no way to convince rev-list not
> >   to print the "commit ..." header, which is really what we're avoiding
> >   here. We probably should have suppressed it with user-formats when
> >   they were introduced, but it's too late to make that change. I wonder
> >   if it would be worth adding a command-line option, though. I've often
> >   had to hack around this when parsing rev-list output (and sometimes
> >   even resort to using git-log if it's a one-off).
> 
> Or make "git log" without frills as fast as rev-list, perhaps?
> 
> What extra things do we do that makes "log" inherently slower than
> "rev-list"?

It's not the speed of log that is a problem, but just that I usually try
to use plumbing when scripting. So I often reach for rev-list first.

I do think for just listing commit hashes that log is slower, though.
One reason is that when there's a commit-graph, it's not as good at
avoiding reading the commit objects. E.g.:

  $ time git rev-list HEAD >/dev/null
  real	0m0.031s
  user	0m0.027s
  sys	0m0.004s

  $ time git -c core.commitgraph=false rev-list HEAD >/dev/null
  real	0m0.362s
  user	0m0.345s
  sys	0m0.016s

  $ time git log --format=%H HEAD >/dev/null
  real	0m0.371s
  user	0m0.355s
  sys	0m0.016s

So running "git log" takes about the same time as rev-list if we disable
the commit-graph. Which makes sense. The pretty-print code is aggressive
about loading the object contents, even if we end up not needing it
(because really, everything _except_ userformat does need it, and even
userformat usually needs it).

I think it would be nice to make the formatting code smarter about
reporting exactly which parts it needs.

> I do not mind a new option (e.g. --no-header) to "rev-list", though.

I took a brief look at this earlier today and it was more awkward than I
expected. The "commit <oid>" header might also have other stuff attached
to it (revision marks, parents, and who even knew we had a "--timestamp"
option?). It's not clear where those things should go if we suppress the
header (for oneline, they just get stuck in front of the oneline; would
that be OK for userformats, too?).

-Peff