From: Christian Couder Date: Sat, 05 Dec 2020 11:10:41 GMT Subject: Re: [PATCH v4 02/19] convert: add [async_]convert_to_working_tree_ca() variants Message-ID: In-Reply-To: On Wed, Nov 4, 2020 at 9:33 PM Matheus Tavares wrote: > > From: Jeff Hostetler > > 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 > [matheus.bernardino: squash, remove one function definition and reword] I'd rather have "Reworked-by: Matheus Tavares " or "Improved-by: Matheus Tavares " than lines such as the above one. > Signed-off-by: Matheus Tavares > @@ -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".