Subject: Re: [PATCH 2alt/2] dir: match "attr" pathspec magic with correct paths

From: Junio C Hamano <[email-masked]>
Date: Sun, 09 Jul 2023 02:28:19 -0700

René Scharfe <[email-masked]> writes:

>> This "common leading prefix" optimization has two main features:
>>
>>  * discard the entries in the in-core index that are outside of the
>>    common leading prefix; if you are doing "ls-files one/a one/b",
>>    we know all matches must be from "one/", so first the code
>>    discards all entries outside the "one/" directory from the
>>    in-core index.  This allows us to work on a smaller dataset.
>>
>>  * allow skipping the comparison of the leading bytes when matching
>>    pathspec with path.  When "ls-files" finds the path "one/a/1" in
>>    the in-core index given "one/a" and "one/b" as the pathspec,
>>    knowing that common leading prefix "one/" was found lets the
>>    pathspec matchinery not to bother comparing "one/" part, and
>>    allows it to feed "a/1" down, as long as the pathspec element
>>    "one/a" gets corresponding adjustment to "a".
>> ...
> With your big patch 2:
> ...
> The difference to main is small enough to get lost in the noise.
>
> The one-line fix is nice and surgical, but I like the other one more.
> Gets rid of crusty underutilized code that doesn't even seem to make
> a measurable difference.

Your benchmark matches my intuition, in that the main benefit of the
optimization comes from discarding the in-core cache entries outside
the area covered by the common prefix, and not from being able to
skip comparing a leading bytes.  The value in code simplification
the larger change has may want to be pursued later, but I'd rather
see us make a small "fix" that can be merged down to 'maint' first.

Thanks.