[v3,09/19] entry: add checkout_entry_ca() which takes preloaded conv_attrs

Commit Message

Matheus Tavares Oct. 29, 2020, 2:14 a.m. UTC

The parallel checkout machinery will call checkout_entry() for entries
that could not be written in parallel due to path collisions. At this
point, we will already be holding the conversion attributes for each
entry, and it would be wasteful to let checkout_entry() load these
again. Instead, let's add the checkout_entry_ca() variant, which
optionally takes a preloaded conv_attrs struct.

Signed-off-by: Matheus Tavares <[e-mail]>
---
 entry.c | 23 ++++++++++++-----------
 entry.h | 13 +++++++++++--
 2 files changed, 23 insertions(+), 13 deletions(-)

Comments

Junio C Hamano Oct. 30, 2020, 10:02 p.m. UTC | #1

Matheus Tavares <[e-mail]> writes:

> The parallel checkout machinery will call checkout_entry() for entries
> that could not be written in parallel due to path collisions. At this
> point, we will already be holding the conversion attributes for each
> entry, and it would be wasteful to let checkout_entry() load these
> again. Instead, let's add the checkout_entry_ca() variant, which
> optionally takes a preloaded conv_attrs struct.
>
> Signed-off-by: Matheus Tavares <[e-mail]>
> ---

I think my review comment to 08/19 is partly taken care of with this
step.  Perhaps the progression will become simpler to understand if
we add this new helper first?  I dunno.  In either case, the end
result of applying both 08 and 09 looks quite nicer than the state
after up to 07 are applied.

Patch

diff --git a/entry.c b/entry.c
index 8237859b12..9d79a5671f 100644
--- a/entry.c
+++ b/entry.c
@@ -440,12 +440,13 @@  static void mark_colliding_entries(const struct checkout *state,
 	}
 }
 
-int checkout_entry(struct cache_entry *ce, const struct checkout *state,
-		   char *topath, int *nr_checkouts)
+int checkout_entry_ca(struct cache_entry *ce, struct conv_attrs *ca,
+		      const struct checkout *state, char *topath,
+		      int *nr_checkouts)
 {
 	static struct strbuf path = STRBUF_INIT;
 	struct stat st;
-	struct conv_attrs ca;
+	struct conv_attrs ca_buf;
 
 	if (ce->ce_flags & CE_WT_REMOVE) {
 		if (topath)
@@ -459,11 +460,11 @@  int checkout_entry(struct cache_entry *ce, const struct checkout *state,
 	}
 
 	if (topath) {
-		if (S_ISREG(ce->ce_mode)) {
-			convert_attrs(state->istate, &ca, ce->name);
-			return write_entry(ce, topath, &ca, state, 1);
+		if (S_ISREG(ce->ce_mode) && !ca) {
+			convert_attrs(state->istate, &ca_buf, ce->name);
+			ca = &ca_buf;
 		}
-		return write_entry(ce, topath, NULL, state, 1);
+		return write_entry(ce, topath, ca, state, 1);
 	}
 
 	strbuf_reset(&path);
@@ -530,12 +531,12 @@  int checkout_entry(struct cache_entry *ce, const struct checkout *state,
 	if (nr_checkouts)
 		(*nr_checkouts)++;
 
-	if (S_ISREG(ce->ce_mode)) {
-		convert_attrs(state->istate, &ca, ce->name);
-		return write_entry(ce, path.buf, &ca, state, 0);
+	if (S_ISREG(ce->ce_mode) && !ca) {
+		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)
diff --git a/entry.h b/entry.h
index 664aed1576..2081fbbbab 100644
--- a/entry.h
+++ b/entry.h
@@ -27,9 +27,18 @@  struct checkout {
  * file named by ce, a temporary file is created by this function and
  * its name is returned in topath[], which must be able to hold at
  * least TEMPORARY_FILENAME_LENGTH bytes long.
+ *
+ * With checkout_entry_ca(), callers can optionally pass a preloaded
+ * conv_attrs struct (to avoid reloading it), when ce refers to a
+ * regular file. If ca is NULL, the attributes will be loaded
+ * internally when (and if) needed.
  */
-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);
+
 void enable_delayed_checkout(struct checkout *state);
 int finish_delayed_checkout(struct checkout *state, int *nr_checkouts);
 /*