| Message ID | 81e03baab1dd7e28262e1d721eac1646c5908b5a.1603937110.git.matheus.bernardino@usp.br (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Parallel Checkout (part I) | expand |
Matheus Tavares <matheus.bernardino@usp.br> writes: > +/* Note: ca is used (and required) iff the entry refers to a regular file. */ This reflects how the current code happens to work, and it is unlikely to change (in other words, I offhand do not think of a reason why attributes may affect checking out a symlink or a submodule), so that's probably OK. I mention this specifically because ... > +static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca, > + const struct checkout *state, int to_tempfile) > { > unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT; > struct delayed_checkout *dco = state->delayed_checkout; > @@ -281,8 +282,7 @@ static int write_entry(struct cache_entry *ce, > clone_checkout_metadata(&meta, &state->meta, &ce->oid); > > if (ce_mode_s_ifmt == S_IFREG) { > - struct stream_filter *filter = get_stream_filter(state->istate, ce->name, > - &ce->oid); > + struct stream_filter *filter = get_stream_filter_ca(ca, &ce->oid); > if (filter && > !streaming_write_entry(ce, path, filter, > state, to_tempfile, > @@ -329,14 +329,17 @@ static int write_entry(struct cache_entry *ce, > * Convert from git internal format to working tree format > */ > if (dco && dco->state != CE_NO_DELAY) { > - ret = async_convert_to_working_tree(state->istate, ce->name, new_blob, > - size, &buf, &meta, dco); > + ret = async_convert_to_working_tree_ca(ca, ce->name, > + new_blob, size, > + &buf, &meta, dco); > if (ret && string_list_has_string(&dco->paths, ce->name)) { > free(new_blob); > goto delayed; > } > - } else > - ret = convert_to_working_tree(state->istate, ce->name, new_blob, size, &buf, &meta); > + } else { > + ret = convert_to_working_tree_ca(ca, ce->name, new_blob, > + size, &buf, &meta); > + } > > if (ret) { > free(new_blob); > @@ -442,6 +445,7 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, > { > static struct strbuf path = STRBUF_INIT; > struct stat st; > + struct conv_attrs ca; > > if (ce->ce_flags & CE_WT_REMOVE) { > if (topath) > @@ -454,8 +458,13 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, > return 0; > } > > - if (topath) > - return write_entry(ce, topath, state, 1); > + if (topath) { > + if (S_ISREG(ce->ce_mode)) { > + convert_attrs(state->istate, &ca, ce->name); > + return write_entry(ce, topath, &ca, state, 1); > + } > + return write_entry(ce, topath, NULL, state, 1); > + } ... it looked somewhat upside-down at the first glance that we decide if lower level routines are allowed to use the ca at this high level in the callchain. But it is the point of this change to lift the point to make the decision to use attributes higher in the callchain, so it would be OK (or "unavoidable"). I wonder if it is worth to avoid early return from the inner block, like this: struct conv_attrs *use_ca = NULL; ... if (topath) { struct conv_attrs ca; if (S_ISREG(...)) { convert_attrs(... &ca ...); use_ca = &ca; } return write_entry(ce, topath, use_ca, state, 1); } which would make it easier to further add code that is common to both regular file and other things before we call write_entry(). The same comment applies to the codepath where a new file gets created in the next hunk. > @@ -517,9 +526,16 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, > return 0; > > create_directories(path.buf, path.len, state); > + > if (nr_checkouts) > (*nr_checkouts)++; > - return write_entry(ce, path.buf, state, 0); > + > + if (S_ISREG(ce->ce_mode)) { > + convert_attrs(state->istate, &ca, ce->name); > + return write_entry(ce, path.buf, &ca, state, 0); > + } > + > + return write_entry(ce, path.buf, NULL, state, 0); > } > > void unlink_entry(const struct cache_entry *ce)
diff --git a/entry.c b/entry.c index 1d2df188e5..8237859b12 100644 --- a/entry.c +++ b/entry.c @@ -263,8 +263,9 @@ void update_ce_after_write(const struct checkout *state, struct cache_entry *ce, } } -static int write_entry(struct cache_entry *ce, - char *path, const struct checkout *state, int to_tempfile) +/* Note: ca is used (and required) iff the entry refers to a regular file. */ +static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca, + const struct checkout *state, int to_tempfile) { unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT; struct delayed_checkout *dco = state->delayed_checkout; @@ -281,8 +282,7 @@ static int write_entry(struct cache_entry *ce, clone_checkout_metadata(&meta, &state->meta, &ce->oid); if (ce_mode_s_ifmt == S_IFREG) { - struct stream_filter *filter = get_stream_filter(state->istate, ce->name, - &ce->oid); + struct stream_filter *filter = get_stream_filter_ca(ca, &ce->oid); if (filter && !streaming_write_entry(ce, path, filter, state, to_tempfile, @@ -329,14 +329,17 @@ static int write_entry(struct cache_entry *ce, * Convert from git internal format to working tree format */ if (dco && dco->state != CE_NO_DELAY) { - ret = async_convert_to_working_tree(state->istate, ce->name, new_blob, - size, &buf, &meta, dco); + ret = async_convert_to_working_tree_ca(ca, ce->name, + new_blob, size, + &buf, &meta, dco); if (ret && string_list_has_string(&dco->paths, ce->name)) { free(new_blob); goto delayed; } - } else - ret = convert_to_working_tree(state->istate, ce->name, new_blob, size, &buf, &meta); + } else { + ret = convert_to_working_tree_ca(ca, ce->name, new_blob, + size, &buf, &meta); + } if (ret) { free(new_blob); @@ -442,6 +445,7 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, { static struct strbuf path = STRBUF_INIT; struct stat st; + struct conv_attrs ca; if (ce->ce_flags & CE_WT_REMOVE) { if (topath) @@ -454,8 +458,13 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, return 0; } - if (topath) - return write_entry(ce, topath, state, 1); + if (topath) { + if (S_ISREG(ce->ce_mode)) { + convert_attrs(state->istate, &ca, ce->name); + return write_entry(ce, topath, &ca, state, 1); + } + return write_entry(ce, topath, NULL, state, 1); + } strbuf_reset(&path); strbuf_add(&path, state->base_dir, state->base_dir_len); @@ -517,9 +526,16 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, return 0; create_directories(path.buf, path.len, state); + if (nr_checkouts) (*nr_checkouts)++; - return write_entry(ce, path.buf, state, 0); + + if (S_ISREG(ce->ce_mode)) { + convert_attrs(state->istate, &ca, ce->name); + return write_entry(ce, path.buf, &ca, state, 0); + } + + return write_entry(ce, path.buf, NULL, state, 0); } void unlink_entry(const struct cache_entry *ce)
In a following patch, checkout_entry() will use conv_attrs to decide whether an entry should be enqueued for parallel checkout or not. But the attributes lookup only happens lower in this call stack. To avoid the unnecessary work of loading the attributes twice, let's move it up to checkout_entry(), and pass the loaded struct down to write_entry(). Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> --- entry.c | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-)