Re: [PATCH v4 02/19] convert: add [async_]convert_to_working_tree_ca() variants

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 4, 2020 at 9:33 PM Matheus Tavares
<matheus.bernardino@xxxxxx> wrote:
>
> From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
>
> Separate the attribute gathering from the actual conversion by adding
> _ca() variants of the conversion functions. These variants receive a
> precomputed 'struct conv_attrs', not relying, thus, on a index state.

s/a/an/

> They will be used in a future patch adding parallel checkout support,
> for two reasons:
>
> - We will already load the conversion attributes in checkout_entry(),
>   before conversion, to decide whether a path is eligible for parallel
>   checkout. Therefore, it would be wasteful to load them again later,
>   for the actual conversion.
>
> - The parallel workers will be responsible for reading, converting and
>   writing blobs to the working tree. They won't have access to the main
>   process' index state, so they cannot load the attributes. Instead,
>   they will receive the preloaded ones and call the _ca() variant of
>   the conversion functions. Furthermore, the attributes machinery is
>   optimized to handle paths in sequential order, so it's better to leave
>   it for the main process, anyway.

Well explained.

> Signed-off-by: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
> [matheus.bernardino: squash, remove one function definition and reword]

<rant++>I'd rather have "Reworked-by: Matheus Tavares
<matheus.bernardino@xxxxxx>" or "Improved-by: Matheus Tavares
<matheus.bernardino@xxxxxx>" than lines such as the above
one.</rant++>

> Signed-off-by: Matheus Tavares <matheus.bernardino@xxxxxx>

> @@ -1447,7 +1447,7 @@ void convert_to_git_filter_fd(const struct index_state *istate,
>         ident_to_git(dst->buf, dst->len, dst, ca.ident);
>  }
>
> -static int convert_to_working_tree_internal(const struct index_state *istate,
> +static int convert_to_working_tree_internal(const struct conv_attrs *ca,

It is still internal, but for consistency it might be better to add
"_ca" to the name of this function too, as we now pass it a "ca"
instead of an "istate".



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux