[PATCH v5 0/9] Parallel Checkout (part I)
- From
Matheus Tavares <matheus.bernardino@usp.br>
- Date
- Dec 16, 2020, 14:50 UTC
- Message-ID
- <cover.1608128666.git.matheus.bernardino@usp.br>
- In-Reply-To
- <cover.1604521275.git.matheus.bernardino@usp.br>
The previous rounds got many great suggestions about patches 1 to 10, but not as many comments on the latter patches. Christian commented that patches 10 and 11 are too long/complex, making the overall series harder to review. So he suggested that I eject patches 10 to 19, and send them later in a separated part. This will hopefully make the series easier to review and move forward. (I also hope to include a desing doc in part 2 to make those two bigger patches more digestible.)
So this part is now composed only of the 9 preparatory patches, which mainly focus on: (1) adding the 'struct conv_attrs' parameter to some convert.c and entry.c functions (to avoid re-loading the attributes during parallel checkout); and (2) making some functions public (for parallel-checkout.c's later use).
Changes since v4:
General: - Removed "[matheus.bernardino: ...]" lines from patches 1 to 4. - Ejected patches 10 to 19, which will now be sent in part 2.
Patch 2: - Fixed typo on commit message (s/on a index/on an index/). - Added "_ca" to convert_to_working_tree_internal()'s name.
Patch 4: - Reworded patch title for better clarity, as suggested by Christian.
Patch 5: - Mentioned about moving a checkout_entry() comment from entry.c to entry.h in the patch's message.
Patch 7: - Make patch's title more explicit and reworded message for clarity.
Patch 8: - Fixed the last call to write_entry() in checkout_entry(): it should pass 'ca', not NULL.
Patch 9: - Defined checkout_entry() -- which is now a wrapper to checkout_entry_ca() -- as a static inline function instead of a macro. - Shortened patch's title.
Note: to see the big picture where these patches should fit in, please check the previous round with the complete series: https://lore.kernel.org/git/cover.1604521275.git.matheus.bernardino@usp.br/
Jeff Hostetler (4): convert: make convert_attrs() and convert structs public convert: add [async_]convert_to_working_tree_ca() variants convert: add get_stream_filter_ca() variant convert: add classification for conv_attrs struct
Matheus Tavares (5): entry: extract a header file for entry.c functions entry: make fstat_output() and read_blob_entry() public entry: extract update_ce_after_write() from write_entry() entry: move conv_attrs lookup up to checkout_entry() entry: add checkout_entry_ca() taking preloaded conv_attrs
apply.c | 1 + builtin/checkout-index.c | 1 + builtin/checkout.c | 1 + builtin/difftool.c | 1 + cache.h | 24 ------- convert.c | 143 ++++++++++++++++++++------------------- convert.h | 96 +++++++++++++++++++++++--- entry.c | 85 +++++++++++++---------- entry.h | 59 ++++++++++++++++ unpack-trees.c | 1 + 10 files changed, 275 insertions(+), 137 deletions(-) create mode 100644 entry.h
Range-diff against v4:
1: 2726f6dc05 ! 1: fa04185237 convert: make convert_attrs() and convert structs public
@@ Commit message
convert_crlf_action, which is more appropriate for the global namespace.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
- [matheus.bernardino: squash and reword msg]
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
## convert.c ##
2: fc03417592 ! 2: 2d3c1dc0a1 convert: add [async_]convert_to_working_tree_ca() variants
@@ Commit message
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.
+ precomputed 'struct conv_attrs', not relying, thus, on an index state.
They will be used in a future patch adding parallel checkout support,
for two reasons:
@@ Commit message
it for the main process, anyway.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
- [matheus.bernardino: squash, remove one function definition and reword]
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
## convert.c ##
@@ convert.c: void convert_to_git_filter_fd(const struct index_state *istate,
}
-static int convert_to_working_tree_internal(const struct index_state *istate,
-+static int convert_to_working_tree_internal(const struct conv_attrs *ca,
- const char *path, const char *src,
- size_t len, struct strbuf *dst,
- int normalizing,
-@@ convert.c: static int convert_to_working_tree_internal(const struct index_state *istate,
- struct delayed_checkout *dco)
+- const char *path, const char *src,
+- size_t len, struct strbuf *dst,
+- int normalizing,
+- const struct checkout_metadata *meta,
+- struct delayed_checkout *dco)
++static int convert_to_working_tree_ca_internal(const struct conv_attrs *ca,
++ const char *path, const char *src,
++ size_t len, struct strbuf *dst,
++ int normalizing,
++ const struct checkout_metadata *meta,
++ struct delayed_checkout *dco)
{
int ret = 0, ret_filter = 0;
- struct conv_attrs ca;
@@ convert.c: static int convert_to_working_tree_internal(const struct index_state
+ void *dco)
{
- return convert_to_working_tree_internal(istate, path, src, len, dst, 0, meta, dco);
-+ return convert_to_working_tree_internal(ca, path, src, len, dst, 0, meta, dco);
++ return convert_to_working_tree_ca_internal(ca, path, src, len, dst, 0,
++ meta, dco);
}
-int convert_to_working_tree(const struct index_state *istate,
@@ convert.c: static int convert_to_working_tree_internal(const struct index_state
+ const struct checkout_metadata *meta)
{
- return convert_to_working_tree_internal(istate, path, src, len, dst, 0, meta, NULL);
-+ return convert_to_working_tree_internal(ca, path, src, len, dst, 0, meta, NULL);
++ return convert_to_working_tree_ca_internal(ca, path, src, len, dst, 0,
++ meta, NULL);
}
int renormalize_buffer(const struct index_state *istate, const char *path,
@@ convert.c: static int convert_to_working_tree_internal(const struct index_state
+ int ret;
+
+ convert_attrs(istate, &ca, path);
-+ ret = convert_to_working_tree_internal(&ca, path, src, len, dst, 1, NULL, NULL);
++ ret = convert_to_working_tree_ca_internal(&ca, path, src, len, dst, 1,
++ NULL, NULL);
if (ret) {
src = dst->buf;
len = dst->len;
3: 8ce20f1031 ! 3: 8af17f6a9b convert: add get_stream_filter_ca() variant
@@ Commit message
attributes struct as a parameter.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
- [matheus.bernardino: move header comment to ca() variant and reword msg]
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
## convert.c ##
4: aa1eb461f4 ! 4: f829e2e08f convert: add conv_attrs classification
@@ Metadata
Author: Jeff Hostetler <jeffhost@microsoft.com>
## Commit message ##
- convert: add conv_attrs classification
+ convert: add classification for conv_attrs struct
Create `enum conv_attrs_classification` to express the different ways
that attributes are handled for a blob during checkout.
@@ Commit message
classifying logic is the same).
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
- [matheus.bernardino: use classification in get_stream_filter_ca()]
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
## convert.c ##
5: cb3dea224b ! 5: 934ead9519 entry: extract a header file for entry.c functions
@@ Commit message
reside in cache.h. Although not many, they contribute to the size of
cache.h and, when changed, cause the unnecessary recompilation of
modules that don't really use these functions. So let's move them to a
- new entry.h header.
+ new entry.h header. While at it let's also move a comment related to
+ checkout_entry() from entry.c to entry.h as it's more useful to describe
+ the function there.
Original-patch-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
6: 46ed6274d7 = 6: da6d1d7624 entry: make fstat_output() and read_blob_entry() public
7: a0479d02ff ! 7: def654606d entry: extract cache_entry update from write_entry()
@@ Metadata
Author: Matheus Tavares <matheus.bernardino@usp.br>
## Commit message ##
- entry: extract cache_entry update from write_entry()
+ entry: extract update_ce_after_write() from write_entry()
- This code will be used by the parallel checkout functions, outside
- entry.c, so extract it to a public function.
+ The code that updates the in-memory index information after an entry is
+ written currently resides in write_entry(). Extract it to a public
+ function so that it can be called by the parallel checkout functions,
+ outside entry.c, in a later patch.
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
8: 5c993cc27f ! 8: fbde901518 entry: move conv_attrs lookup up to checkout_entry()
@@ entry.c: int checkout_entry(struct cache_entry *ce, const struct checkout *state
+ ca = &ca_buf;
+ }
+
-+ return write_entry(ce, path.buf, NULL, state, 0);
++ return write_entry(ce, path.buf, ca, state, 0);
}
void unlink_entry(const struct cache_entry *ce)
9: aa635bda21 ! 9: 0556d32e8c entry: add checkout_entry_ca() which takes preloaded conv_attrs
@@ Metadata
Author: Matheus Tavares <matheus.bernardino@usp.br>
## Commit message ##
- entry: add checkout_entry_ca() which takes preloaded conv_attrs
+ entry: add checkout_entry_ca() taking preloaded conv_attrs
The parallel checkout machinery will call checkout_entry() for entries
that could not be written in parallel due to path collisions. At this
@@ entry.c: int checkout_entry(struct cache_entry *ce, const struct checkout *state
convert_attrs(state->istate, &ca_buf, ce->name);
ca = &ca_buf;
}
-
-- return write_entry(ce, path.buf, NULL, state, 0);
-+ return write_entry(ce, path.buf, ca, state, 0);
- }
-
- void unlink_entry(const struct cache_entry *ce)
## entry.h ##
@@ entry.h: struct checkout {
@@ entry.h: struct checkout {
*/
-int checkout_entry(struct cache_entry *ce, const struct checkout *state,
- char *topath, int *nr_checkouts);
-+#define checkout_entry(ce, state, topath, nr_checkouts) \
-+ checkout_entry_ca(ce, NULL, state, topath, nr_checkouts)
+int checkout_entry_ca(struct cache_entry *ce, struct conv_attrs *ca,
+ const struct checkout *state, char *topath,
+ int *nr_checkouts);
++static inline int checkout_entry(struct cache_entry *ce,
++ const struct checkout *state, char *topath,
++ int *nr_checkouts)
++{
++ return checkout_entry_ca(ce, NULL, state, topath, nr_checkouts);
++}
void enable_delayed_checkout(struct checkout *state);
int finish_delayed_checkout(struct checkout *state, int *nr_checkouts);
10: bc8447cd9c < -: ---------- unpack-trees: add basic support for parallel checkout
11: 815137685a < -: ---------- parallel-checkout: make it truly parallel
12: 2b42621582 < -: ---------- parallel-checkout: support progress displaying
13: 960116579a < -: ---------- make_transient_cache_entry(): optionally alloc from mem_pool
14: fb9f2f580c < -: ---------- builtin/checkout.c: complete parallel checkout support
15: a844451e58 < -: ---------- checkout-index: add parallel checkout support
16: 3733857ffa < -: ---------- parallel-checkout: add tests for basic operations
17: c8a2974f81 < -: ---------- parallel-checkout: add tests related to clone collisions
18: 86fccd57d5 < -: ---------- parallel-checkout: add tests related to .gitattributes
19: 7f3e23cc38 < -: ---------- ci: run test round with parallel-checkout enabled-- 2.29.2