From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2alt/2] dir: match "attr" pathspec magic with correct paths
Date: Sun, 09 Jul 2023 02:28:19 -0700 [thread overview]
Message-ID: <xmqqzg454eoc.fsf@gitster.g> (raw)
In-Reply-To: 39d5e14d-f2e9-e619-c3e8-e63c7547370b@web.de
René Scharfe <l.s.r@web.de> 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.
next prev parent reply other threads:[~2023-07-09 9:30 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-07 22:04 [PATCH 0/2] Fix attr magic combined with pathspec prefix Junio C Hamano
2023-07-07 22:04 ` [PATCH 1/2] t6135: attr magic with path pattern Junio C Hamano
2023-07-07 22:04 ` [PATCH 2/2] dir: do not feed path suffix to pathspec match Junio C Hamano
2023-07-07 23:45 ` [PATCH 2/2alt] " Junio C Hamano
2023-07-08 7:16 ` René Scharfe
2023-07-08 21:35 ` [PATCH 2alt/2] dir: match "attr" pathspec magic with correct paths Junio C Hamano
2023-07-09 5:35 ` René Scharfe
2023-07-09 9:28 ` Junio C Hamano [this message]
2023-07-08 7:16 ` [PATCH 2/2] dir: do not feed path suffix to pathspec match René Scharfe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqzg454eoc.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=l.s.r@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.