Re: [PATCH 3/4] pathspec: apply "*.c" optimization from exclude
- From
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
- Date
- Nov 20, 2012, 13:23 UTC
- Message-ID
- <CACsJy8C=vdvHYgMr8Qa7M+Mq=QO6p6dsOFm_ZW=Nf2VQrPwaMg@mail.gmail.com>
- In-Reply-To
- <7vtxsluw3k.fsf@alter.siamese.dyndns.org>
On Tue, Nov 20, 2012 at 4:20 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
>> $ time git rev-list --quiet HEAD -- '*.c' >> >> real 0m40.770s >> user 0m40.290s >> sys 0m0.256s >> >> With the patch >> >> $ time ~/w/git/git rev-list --quiet HEAD -- '*.c' >> >> real 0m34.288s >> user 0m33.997s >> sys 0m0.205s >> >> The above command is not supposed to be widely popular. > > Hrm, perhaps. I use "git grep <pattern> -- \*.c" quite a lot, but > haven't seen use case for \*.c in the context of the "log" family.
"git diff *.c" is also helpful (maybe "git diff *.[ch]" is more often used). But I suspect I/O dominates in both grep and diff cases. I just try to make sure matching code won't show up in profile.
Show 5 quoted lines
>> +#define PSF_ONESTAR 1 > > Together with the GF_ prefix in the previous, PSF_ prefix needs a > bit of in-code explanation. Is it just an RC3L (random combination > of 3 letters?)
I'm pretty sure "PS" stands for pathspec. "F" is probably from fnmatch. Any suggestions?
Show 22 quoted lines
>> @@ -46,6 +46,12 @@ inline int git_fnmatch(const char *pattern, const char *string,
>> pattern += prefix;
>> string += prefix;
>> }
>> + if (flags & GF_ONESTAR) {
>> + int pattern_len = strlen(++pattern);
>> + int string_len = strlen(string);
>> + return strcmp(pattern,
>> + string + string_len - pattern_len);
>> + }
>
> What happens when pattern="foo*oob" and string="foob"?
>
> The prefix match before this code determines that the literal prefix
> in the pattern matches with the early part of the string, and makes
> pattern="*oob" and string="b".
>
> When you come to strcmp(), you see that string_len is 1, pattern_len
> is 3, and pattern is "oob". string+string_len-pattern_len = "oob",
> one past the beginning of the original string "foob". They match.
>
> Oops?Oops indead. I'll need to check exclude code too :(
> return (string_len < pattern_len) || > strcmp(pattern, string + string_len - pattern_len); > > perhaps?
Yeah.
-- Duy