Re: [PATCH v2 2/8] builtin/checkout.c: complete parallel checkout support

From: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date: May 3, 2021, 14:21 UTC
Message-ID: <CAHd-oW57tFTQtqKPhS7J=52yhcatd9wQh3sTHVAmjwvPdWw=HA@mail.gmail.com>
In-Reply-To: <CAP8UFD16N3MiYawrRLCxsrYuo4KkvD3SS5F4UqyEF37j4T9yVA@mail.gmail.com>

On Sat, May 1, 2021 at 2:08 PM Christian Couder <christian.couder@gmail.com> wrote:

Show 12 quoted lines

>
> On Fri, Apr 30, 2021 at 11:40 PM Matheus Tavares
> <matheus.bernardino@usp.br> wrote:
> >
> > Pathspec-limited checkouts (like `git checkout *.txt`) are performed by
> > a code path that doesn't yet support parallel checkout because it calls
> > checkout_entry() directly, instead of unpack_trees(). Let's add parallel
> > checkout support for this code path too.
> >
> > Note: the transient cache entries allocated in checkout_merged() are now
>
> s/Note: the/The/

Thanks

Show 9 quoted lines

> > allocated in a mem_pool which is only discarded after parallel checkout
> > finishes. This is done because the entries need to be valid when
> > run_parallel_checkout() is called.
>
> > -static int checkout_merged(int pos, const struct checkout *state, int *nr_checkouts)
> > +static int checkout_merged(int pos, const struct checkout *state,
> > +                          int *nr_checkouts, struct mem_pool *ce_mem_pool)
>
> For consistency with the previous patch, maybe: s/ce_mem_pool/ce_mp/

Yeah, I agree that it's a good idea to keep this consistent. In fact, instead of changing `ce_mem_pool` here, I think I should change `mp` in the previous patch to either `mem_pool` or `ce_mem_pool`. Those seem to be the most common names, at read-cache.c, for a memory pool struct holding cache entries.