git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 27/35] attr: keep attr stack for each check

From
Stefan Beller <sbeller@google.com>
Date
Nov 10, 2016, 20:34 UTC
Message-ID
<20161110203428.30512-28-sbeller@google.com>
In-Reply-To
<20161110203428.30512-1-sbeller@google.com>

Instead of having a global attr stack, attach the stack to each check. This allows to use the attr in a multithreaded way.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 attr.c    | 101 +++++++++++++++++++++++++++++++++++++++-----------------------
 attr.h    |   4 ++-
 hashmap.h |   2 ++
 3 files changed, 69 insertions(+), 38 deletions(-)
diff --git a/attr.c b/attr.c
index 32ec453..979bd3c 100644
--- a/attr.c
+++ b/attr.c
@@ -375,15 +375,17 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
  * .gitignore file and info/excludes file as a fallback.
  */
 
-/* NEEDSWORK: This will become per git_attr_check */
-static struct attr_stack {
+struct attr_stack {
 	struct attr_stack *prev;
 	char *origin;
 	size_t originlen;
 	unsigned num_matches;
 	unsigned alloc;
 	struct match_attr **attrs;
-} *attr_stack;
+};
+
+static struct hashmap all_attr_stacks;
+static int all_attr_stacks_init;
 
 static void free_attr_elem(struct attr_stack *e)
 {
@@ -564,11 +566,23 @@ static void debug_set(const char *what, const char *match, struct git_attr *attr
 
 static void drop_attr_stack(void)
 {
-	while (attr_stack) {
-		struct attr_stack *elem = attr_stack;
-		attr_stack = elem->prev;
-		free_attr_elem(elem);
+	struct hashmap_iter iter;
+	struct git_attr_check *check;
+
+	attr_lock();
+	if (!all_attr_stacks_init) {
+		attr_unlock();
+		return;
 	}
+	hashmap_iter_init(&all_attr_stacks, &iter);
+	while ((check = hashmap_iter_next(&iter))) {
+		while (check->attr_stack) {
+			struct attr_stack *elem = check->attr_stack;
+			check->attr_stack = elem->prev;
+			free_attr_elem(elem);
+		}
+	}
+	attr_unlock();
 }
 
 static const char *git_etc_gitattributes(void)
@@ -598,40 +612,42 @@ static void push_stack(struct attr_stack **attr_stack_p,
 	}
 }
 
-static void bootstrap_attr_stack(void)
+static void bootstrap_attr_stack(struct git_attr_check *check)
 {
 	struct attr_stack *elem;
 
-	if (attr_stack)
+	if (check->attr_stack)
 		return;
 
-	push_stack(&attr_stack, read_attr_from_array(builtin_attr), NULL, 0);
+	push_stack(&check->attr_stack,
+		   read_attr_from_array(builtin_attr), NULL, 0);
 
 	if (git_attr_system())
-		push_stack(&attr_stack,
+		push_stack(&check->attr_stack,
 			   read_attr_from_file(git_etc_gitattributes(), 1),
 			   NULL, 0);
 
 	if (!git_attributes_file)
 		git_attributes_file = xdg_config_home("attributes");
 	if (git_attributes_file)
-		push_stack(&attr_stack,
+		push_stack(&check->attr_stack,
 			   read_attr_from_file(git_attributes_file, 1),
 			   NULL, 0);
 
 	if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
 		elem = read_attr(GITATTRIBUTES_FILE, 1);
-		push_stack(&attr_stack, elem, xstrdup(""), 0);
+		push_stack(&check->attr_stack, elem, xstrdup(""), 0);
 		debug_push(elem);
 	}
 
 	elem = read_attr_from_file(git_path_info_attributes(), 1);
 	if (!elem)
 		elem = xcalloc(1, sizeof(*elem));
-	push_stack(&attr_stack, elem, NULL, 0);
+	push_stack(&check->attr_stack, elem, NULL, 0);
 }
 
-static void prepare_attr_stack(const char *path, int dirlen)
+static void prepare_attr_stack(const char *path, int dirlen,
+			       struct git_attr_check *check)
 {
 	struct attr_stack *elem, *info;
 	const char *cp;
@@ -651,13 +667,13 @@ static void prepare_attr_stack(const char *path, int dirlen)
 	 * .gitattributes in deeper directories to shallower ones,
 	 * and finally use the built-in set as the default.
 	 */
-	bootstrap_attr_stack();
+	bootstrap_attr_stack(check);
 
 	/*
 	 * Pop the "info" one that is always at the top of the stack.
 	 */
-	info = attr_stack;
-	attr_stack = info->prev;
+	info = check->attr_stack;
+	check->attr_stack = info->prev;
 
 	/*
 	 * Pop the ones from directories that are not the prefix of
@@ -665,17 +681,17 @@ static void prepare_attr_stack(const char *path, int dirlen)
 	 * the root one (whose origin is an empty string "") or the builtin
 	 * one (whose origin is NULL) without popping it.
 	 */
-	while (attr_stack->origin) {
-		int namelen = strlen(attr_stack->origin);
+	while (check->attr_stack->origin) {
+		int namelen = strlen(check->attr_stack->origin);
 
-		elem = attr_stack;
+		elem = check->attr_stack;
 		if (namelen <= dirlen &&
 		    !strncmp(elem->origin, path, namelen) &&
 		    (!namelen || path[namelen] == '/'))
 			break;
 
 		debug_pop(elem);
-		attr_stack = elem->prev;
+		check->attr_stack = elem->prev;
 		free_attr_elem(elem);
 	}
 
@@ -691,9 +707,9 @@ static void prepare_attr_stack(const char *path, int dirlen)
 		 */
 		struct strbuf pathbuf = STRBUF_INIT;
 
-		assert(attr_stack->origin);
+		assert(check->attr_stack->origin);
 		while (1) {
-			size_t len = strlen(attr_stack->origin);
+			size_t len = strlen(check->attr_stack->origin);
 			char *origin;
 
 			if (dirlen <= len)
@@ -707,7 +723,7 @@ static void prepare_attr_stack(const char *path, int dirlen)
 			elem = read_attr(pathbuf.buf, 0);
 			strbuf_setlen(&pathbuf, cp - path);
 			origin = strbuf_detach(&pathbuf, &len);
-			push_stack(&attr_stack, elem, origin, len);
+			push_stack(&check->attr_stack, elem, origin, len);
 			debug_push(elem);
 		}
 
@@ -717,7 +733,13 @@ static void prepare_attr_stack(const char *path, int dirlen)
 	/*
 	 * Finally push the "info" one at the top of the stack.
 	 */
-	push_stack(&attr_stack, info, NULL, 0);
+	push_stack(&check->attr_stack, info, NULL, 0);
+	if (!all_attr_stacks_init) {
+		hashmap_init(&all_attr_stacks, NULL, 0);
+		all_attr_stacks_init = 1;
+	}
+	if (!hashmap_get(&all_attr_stacks, check, NULL))
+		hashmap_put(&all_attr_stacks, check);
 }
 
 static int path_matches(const char *pathname, int pathlen,
@@ -743,9 +765,10 @@ static int path_matches(const char *pathname, int pathlen,
 			      pattern, prefix, pat->patternlen, pat->flags);
 }
 
-static int macroexpand_one(int attr_nr, int rem);
+static int macroexpand_one(int attr_nr, int rem, struct git_attr_check *check);
 
-static int fill_one(const char *what, struct match_attr *a, int rem)
+static int fill_one(const char *what, struct match_attr *a, int rem,
+		    struct git_attr_check *check)
 {
 	struct git_attr_check_elem *celem = check_all_attr;
 	int i;
@@ -761,14 +784,14 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
 				  attr, v);
 			*n = v;
 			rem--;
-			rem = macroexpand_one(attr->attr_nr, rem);
+			rem = macroexpand_one(attr->attr_nr, rem, check);
 		}
 	}
 	return rem;
 }
 
 static int fill(const char *path, int pathlen, int basename_offset,
-		struct attr_stack *stk, int rem)
+		struct attr_stack *stk, int rem, struct git_attr_check *check)
 {
 	int i;
 	const char *base = stk->origin ? stk->origin : "";
@@ -779,12 +802,12 @@ static int fill(const char *path, int pathlen, int basename_offset,
 			continue;
 		if (path_matches(path, pathlen, basename_offset,
 				 &a->u.pat, base, stk->originlen))
-			rem = fill_one("fill", a, rem);
+			rem = fill_one("fill", a, rem, check);
 	}
 	return rem;
 }
 
-static int macroexpand_one(int nr, int rem)
+static int macroexpand_one(int nr, int rem, struct git_attr_check *check)
 {
 	struct attr_stack *stk;
 	int i;
@@ -793,13 +816,13 @@ static int macroexpand_one(int nr, int rem)
 	    !check_all_attr[nr].attr->maybe_macro)
 		return rem;
 
-	for (stk = attr_stack; stk; stk = stk->prev) {
+	for (stk = check->attr_stack; stk; stk = stk->prev) {
 		for (i = stk->num_matches - 1; 0 <= i; i--) {
 			struct match_attr *ma = stk->attrs[i];
 			if (!ma->is_macro)
 				continue;
 			if (ma->u.attr->attr_nr == nr)
-				return fill_one("expand", ma, rem);
+				return fill_one("expand", ma, rem, check);
 		}
 	}
 
@@ -848,7 +871,7 @@ static void collect_some_attrs(const char *path, int pathlen,
 		dirlen = 0;
 	}
 
-	prepare_attr_stack(path, dirlen);
+	prepare_attr_stack(path, dirlen, check);
 
 	for (i = 0; i < attr_nr; i++)
 		check_all_attr[i].value = ATTR__UNKNOWN;
@@ -868,8 +891,8 @@ static void collect_some_attrs(const char *path, int pathlen,
 	}
 
 	rem = attr_nr;
-	for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
-		rem = fill(path, pathlen, basename_offset, stk, rem);
+	for (stk = check->attr_stack; 0 < rem && stk; stk = stk->prev)
+		rem = fill(path, pathlen, basename_offset, stk, rem, check);
 
 	if (collect_all) {
 		int check_nr = 0, check_alloc = 0;
@@ -902,6 +925,8 @@ static int git_check_attrs(const char *path, int pathlen,
 {
 	int i;
 
+	attr_lock();
+
 	collect_some_attrs(path, pathlen, check, &result, 0);
 
 	for (i = 0; i < check->check_nr; i++) {
@@ -911,6 +936,8 @@ static int git_check_attrs(const char *path, int pathlen,
 		result[i].value = value;
 	}
 
+	attr_unlock();
+
 	return 0;
 }
 
diff --git a/attr.h b/attr.h
index 1a50288..ab381ae 100644
--- a/attr.h
+++ b/attr.h
@@ -30,12 +30,14 @@ extern const char git_attr__false[];
 #define ATTR_UNSET(v) ((v) == NULL)
 
 struct git_attr_check {
+	struct hashmap_entry entry;
 	int finalized;
 	int check_nr;
 	int check_alloc;
 	const struct git_attr **attr;
+	struct attr_stack *attr_stack;
 };
-#define GIT_ATTR_CHECK_INIT {0, 0, 0, NULL}
+#define GIT_ATTR_CHECK_INIT {HASHMAP_ENTRY_INIT, 0, 0, 0, NULL, NULL}
 
 struct git_attr_result {
 	const char *value;
diff --git a/hashmap.h b/hashmap.h
index ab7958a..d247c62 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -31,6 +31,8 @@ struct hashmap_entry {
 	unsigned int hash;
 };
 
+#define HASHMAP_ENTRY_INIT {NULL, 0}
+
 typedef int (*hashmap_cmp_fn)(const void *entry, const void *entry_or_key,
 		const void *keydata);
 
-- 
2.10.1.469.g00a8914
Previous: Stefan BellerNext: Stefan Beller
Message 19 of 43 in “[PATCHv3 00/35] Revamp the attr subsystem!”
  1. Stefan BellerNov 10, 2016
  2. 02/35 attr.c: use strchrnul() to scan for one lineStefan Beller, Nov 10, 2016
  3. 06/35 attr.c: mark where #if DEBUG ends more clearlyStefan Beller, Nov 10, 2016
  4. 05/35 attr.c: complete a sentence in a commentStefan Beller, Nov 10, 2016
  5. 09/35 attr.c: plug small leak in parse_attr_line()Stefan Beller, Nov 10, 2016
  6. 10/35 attr: rename function and struct related to checking attributesStefan Beller, Nov 10, 2016
  7. 12/35 attr: convert git_all_attrs() to use "struct git_attr_check"Stefan Beller, Nov 10, 2016
  8. 11/35 attr: (re)introduce git_check_attr() and struct git_attr_checkStefan Beller, Nov 10, 2016
  9. 13/35 attr: convert git_check_attrs() callers to use the new APIStefan Beller, Nov 10, 2016
  10. 18/35 attr.c: add push_stack() helperStefan Beller, Nov 10, 2016
  11. 20/35 attr.c: rename a local variable checkStefan Beller, Nov 10, 2016
  12. 22/35 attr.c: introduce empty_attr_check_elems()Stefan Beller, Nov 10, 2016
  13. 19/35 attr.c: pass struct git_attr_check down the callchainStefan Beller, Nov 10, 2016
  14. 17/35 attr: support quoting pathname patterns in C styleStefan Beller, Nov 10, 2016
  15. 16/35 attr: expose validity check for attribute namesStefan Beller, Nov 10, 2016
  16. 34/35 clone: add --init-submodule=<pathspec> switchStefan Beller, Nov 10, 2016
  17. 26/35 attr: convert to new threadsafe APIStefan Beller, Nov 10, 2016
  18. 35/35 completion: clone can initialize specific submodulesStefan Beller, Nov 10, 2016
  19. 27/35 attr: keep attr stack for each checkStefan Beller, Nov 10, 2016
  20. 33/35 submodule update: add `--init-default-path` switchStefan Beller, Nov 10, 2016
  21. 28/35 Documentation: fix a typoStefan Beller, Nov 10, 2016
  22. 31/35 pathspec: allow querying for attributesStefan Beller, Nov 10, 2016
  23. Duy NguyenNov 22, 2016
  24. Stefan BellerNov 22, 2016
  25. Duy NguyenNov 23, 2016
  26. Brandon WilliamsNov 28, 2016
  27. Brandon WilliamsNov 28, 2016
  28. Brandon WilliamsNov 28, 2016
  29. Stefan BellerNov 29, 2016
  30. 32/35 pathspec: allow escaped query valuesStefan Beller, Nov 10, 2016
  31. 30/35 pathspec: move prefix check out of the inner loopStefan Beller, Nov 10, 2016
  32. 29/35 pathspec: move long magic parsing out of prefix_pathspecStefan Beller, Nov 10, 2016
  33. 23/35 attr.c: always pass check[] to collect_some_attrs()Stefan Beller, Nov 10, 2016
  34. 25/35 attr: make git_check_attr_counted staticStefan Beller, Nov 10, 2016
  35. 24/35 attr.c: outline the future plans by heavily commentingStefan Beller, Nov 10, 2016
  36. 21/35 attr.c: correct ugly hack for git_all_attrs()Stefan Beller, Nov 10, 2016
  37. 14/35 attr: retire git_check_attrs() APIStefan Beller, Nov 10, 2016
  38. 01/35 commit.c: use strchrnul() to scan for one lineStefan Beller, Nov 10, 2016
  39. 15/35 attr: add counted string version of git_check_attr()Stefan Beller, Nov 10, 2016
  40. 08/35 attr.c: tighten constness around "git_attr" structureStefan Beller, Nov 10, 2016
  41. 03/35 attr.c: update a stale comment on "struct match_attr"Stefan Beller, Nov 10, 2016
  42. 07/35 attr.c: simplify macroexpand_one()Stefan Beller, Nov 10, 2016
  43. 04/35 attr.c: explain the lack of attr-name syntax check in parse_attr()Stefan Beller, Nov 10, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.