Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Sep 29, 2012, 06:13 UTC
Message-ID: <CACsJy8A1UroqCezJFjqOqBQg+puX=jc1Q-CMSB4f=CHULY=OyA@mail.gmail.com>
In-Reply-To: <7vpq55idbe.fsf@alter.siamese.dyndns.org>

On Sat, Sep 29, 2012 at 12:54 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

>> I like how callers not doing a reflog walk do not have to pay the price
>> to do the extra allocating. We could further limit it to only when
>> --grep-reflog is in effect, but I guess that would mean wading through
>> grep_filter's patterns, since it could be buried amidst ANDs and ORs?
>>
>> One alternative would be to set a bit in the grep_opt when we call
>> append_header_grep_pattern. It feels a bit like a layering violation,
>> though. I guess the bit could also go into rev_info. It may not even be
>> a measurable slowdown, though. Premature optimization and all that.
>
> I do not think it is a layering violation.  compile_grep_exp()
> should be aware of the short-cut possibilities and your "our
> expression is interested in reflog so we need to read it" is very
> similar in spirit to the existing opt->extended bit.
>
> It will obviously allow us to avoid reading reflog information
> unnecessarily here.  I think it makes perfect sense.

reflog, in terms of both the number of commits and message length, is usually short enough that slowdown does not really show, especially when used with git-log, an interactive command.

Without the changes:

$ time git log -g --grep . >/dev/null

real 0m0.480s user 0m0.451s sys 0m0.025s

With the changes:

$ time ./git log -g --grep . >/dev/null

real 0m0.490s user 0m0.471s sys 0m0.018s

> We may also want to flag the use of the --grep-reflog option when
> the --walk-reflogs option is not in effect in setup_revisions() as
> an error, or something.

That's why I put "Ignored unless --walk-reflogs is given" in the document. But an error would be fine too. I suppose an error is preferable in case users do not read document carefully?

-- 
Duy