Re: [PATCH v2] name-hash: don't add sparse directories in threaded lazy init

From: Alex Mironov <alexandrfox@gmail.com>
Date: May 21, 2025, 20:37 UTC
Message-ID: <CAC97EbxRpG1ecWcQ=yJHnqe7gXYG7BNo1sF9e9Kf-EOkDySfpw@mail.gmail.com>
In-Reply-To: <xmqqecwhaemu.fsf@gitster.g>

Hey Junio,

With respect to messaging I more or less copy-pasted Derricks message from the original commit for non-threaded init: please check the referenced commit. Let me know if another wording is needed/preferred.

>  Also, can we have performance numbers in the

proposed log message as well, or is the improvement too small to measure?

In their current form benchmarks do not show major improvements outside of special casing I mentioned in my previous message (existence of files/objects outside of sparse cones in special VFS setups). So, for the general audience this indeed can be treated as a correctness fix.

On Wed, May 21, 2025 at 10:32 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

>
> "Alex Mironov via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Alex Mironov <alexandrfox@gmail.com>
> >
> > Ensure that logic added in 5f11669586 (name-hash: don't add directories
> > to name_hash, 2021-04-12) also applies in multithreaded hashtable init
> > path.
> >
> > Sparse directory entries represent a directory that is outside the
> > sparse-checkout definition. These are not paths to blobs, so should not
> > be added to the name_hash table as they must never be queried.
>
> The second paragraph sounds as if this is a correctness fix,
> i.e. "should not be added" hints that we would see a wrong result
> returned from the hashmap if you added these entries to the
> name_hash.  If this is a performance-only fix, that should be more
> clearly stated.  Also, can we have performance numbers in the
> proposed log message as well, or is the improvement too small to
> measure?
>
> Thanks.

-- 
Best,
Alex Mironov