Performance improvement: Switch phantom symlink list for trie

compat/mingw.c

@@ -9,6 +9,7 @@
 #include "dir.h"
 #include "environment.h"
 #include "gettext.h"
+#include "hashmap.h"
 #include "repository.h"
 #include "run-command.h"
 #include "strbuf.h"
@@ -448,36 +449,241 @@ process_phantom_symlink(const wchar_t *wtarget, const wchar_t *wlink)
 	return PHANTOM_SYMLINK_RETRY;
 }
 
-/* keep track of newly created symlinks to non-existing targets */
+/*
+ * Newly created symlinks to non-existing targets are indexed by a trie
+ * over their target path's components, so mkdir() (or a symlink turning
+ * into a directory symlink) can wake only the entries nested under the
+ * path that just became traversable, instead of re-probing everything.
+ */
 struct phantom_symlink_info {
 	struct phantom_symlink_info *next;
 	wchar_t *wlink;
 	wchar_t *wtarget;
 };
 
-static struct phantom_symlink_info *phantom_symlinks = NULL;
+struct phantom_trie_node {
+	struct hashmap_entry ent;
+	char *component;
+	struct hashmap children;
+	struct phantom_symlink_info *waiters;
+};
+
 static CRITICAL_SECTION phantom_symlinks_cs;
 
-static void process_phantom_symlinks(void)
+static int phantom_trie_node_cmp(const void *cmp_data UNUSED,
+				  const struct hashmap_entry *eptr,
+				  const struct hashmap_entry *entry_or_key,
+				  const void *keydata)
+{
+	const struct phantom_trie_node *e =
+		container_of(eptr, const struct phantom_trie_node, ent);
+
+	return !fspatheq(e->component, keydata ? keydata :
+		container_of(entry_or_key, const struct phantom_trie_node,
+			     ent)->component);
+}
+
+static struct phantom_trie_node phantom_trie_root =
+	{ .children = HASHMAP_INIT(phantom_trie_node_cmp, NULL) };
+
+static char *phantom_symlink_canonicalize(const wchar_t *wpath)
+{
+	wchar_t wfullpath[MAX_LONG_PATH];
+	char utf8[MAX_LONG_PATH * 3];
+	int len = GetFullPathNameW(wpath, ARRAY_SIZE(wfullpath), wfullpath, NULL);
+
+	if (!len || len >= ARRAY_SIZE(wfullpath) ||
+	    xwcstoutf(utf8, wfullpath, sizeof(utf8)) < 0)
+		return NULL;
+	return xstrdup(utf8);
+}
+
+/* the path process_phantom_symlink() itself probes with CreateFileW() */
+static char *phantom_symlink_target_key(const wchar_t *wtarget,
+					 const wchar_t *wlink)
+{
+	wchar_t relative[MAX_LONG_PATH];
+	const wchar_t *rel = make_relative_to(wtarget, wlink, relative,
+					       ARRAY_SIZE(relative));
+
+	return rel ? phantom_symlink_canonicalize(rel) : NULL;
+}
+
+static struct phantom_trie_node *phantom_trie_child(
+	struct phantom_trie_node *node, const char *component,
+	size_t len, int create)
+{
+	char buf[MAX_LONG_PATH * 3];
+	struct hashmap_entry key;
+	struct phantom_trie_node *child;
+
+	if (!len || len >= sizeof(buf))
+		return NULL;
+	memcpy(buf, component, len);
+	buf[len] = '\0';
+
+	hashmap_entry_init(&key, fspathhash(buf));
+	child = hashmap_get_entry_from_hash(&node->children, key.hash, buf,
+					     struct phantom_trie_node, ent);
+	if (!child && create) {
+		child = xcalloc(1, sizeof(*child));
+		child->component = xstrdup(buf);
+		hashmap_init(&child->children, phantom_trie_node_cmp, NULL, 0);
+		hashmap_entry_init(&child->ent, key.hash);
+		hashmap_add(&node->children, &child->ent);
+	}
+	return child;
+}
+
+static struct phantom_trie_node *phantom_trie_walk(const char *path, int create)
+{
+	struct phantom_trie_node *node = &phantom_trie_root;
+	const char *p = path;
+
+	while (*p && node) {
+		const char *start;
+
+		while (*p == '/' || *p == '\\')
+			p++;
+		start = p;
+		while (*p && *p != '/' && *p != '\\')
+			p++;
+		if (p == start)
+			break;
+		node = phantom_trie_child(node, start, (size_t)(p - start), create);
+	}
+	return node;
+}
+
+static void phantom_trie_wake_node(struct phantom_trie_node *node);
+static void phantom_symlink_directory_resolved(const wchar_t *wtarget,
+						const wchar_t *wlink);
+
+/*
+ * Moves everything nested under `from` (not `from` itself) to the same
+ * relative position under `to`. Used when a symlink at `from`'s path
+ * turns out to point at `to`'s path: anything registered through the
+ * symlink can then still be found by mkdir() under the real path it
+ * points at, not just under the symlink's own path.
+ * assumes phantom_symlinks_cs is held
+ */
+static void phantom_trie_graft_children(struct phantom_trie_node *to,
+					 struct phantom_trie_node *from)
+{
+	struct hashmap_iter iter;
+	struct phantom_trie_node *from_child;
+
+	hashmap_iter_init(&from->children, &iter);
+	while ((from_child = container_of_or_null(hashmap_iter_next(&iter),
+						   struct phantom_trie_node, ent))) {
+		struct phantom_trie_node *to_child = phantom_trie_child(
+			to, from_child->component, strlen(from_child->component), 1);
+		if (!to_child)
+			continue;
+		if (from_child->waiters) {
+			struct phantom_symlink_info *last = from_child->waiters;
+
+			while (last->next)
+				last = last->next;
+			last->next = to_child->waiters;
+			to_child->waiters = from_child->waiters;
+			from_child->waiters = NULL;
+		}
+		phantom_trie_graft_children(to_child, from_child);
+	}
+}
+
+/* assumes phantom_symlinks_cs is held */
+static void phantom_trie_drain_waiters(struct phantom_trie_node *node)
 {
 	struct phantom_symlink_info *current, **psi;
-	EnterCriticalSection(&phantom_symlinks_cs);
-	/* process phantom symlinks list */
-	psi = &phantom_symlinks;
-	while ((current = *psi)) {
-		enum phantom_symlink_result result = process_phantom_symlink(
-				current->wtarget, current->wlink);
-		if (result == PHANTOM_SYMLINK_RETRY) {
-			psi = &current->next;
-		} else {
-			/* symlink was processed, remove from list */
+	int restart;
+
+	do {
+		restart = 0;
+		psi = &node->waiters;
+		while ((current = *psi)) {
+			enum phantom_symlink_result result =
+				process_phantom_symlink(current->wtarget, current->wlink);
+
+			if (result == PHANTOM_SYMLINK_RETRY) {
+				psi = &current->next;
+				continue;
+			}
+
 			*psi = current->next;
-			free(current);
-			/* if symlink was a directory, start over */
 			if (result == PHANTOM_SYMLINK_DIRECTORY)
-				psi = &phantom_symlinks;
+				phantom_symlink_directory_resolved(current->wtarget,
+								    current->wlink);
+			free(current);
+
+			if (result == PHANTOM_SYMLINK_DIRECTORY) {
+				/* may wake into this same node; restart, don't trust *psi */
+				restart = 1;
+				break;
+			}
 		}
-	}
+	} while (restart);
+}
+
+/* assumes phantom_symlinks_cs is held */
+static void phantom_trie_wake_children(struct phantom_trie_node *node)
+{
+	struct hashmap_iter iter;
+	struct phantom_trie_node *child;
+
+	hashmap_iter_init(&node->children, &iter);
+	while ((child = container_of_or_null(hashmap_iter_next(&iter),
+					      struct phantom_trie_node, ent)))
+		phantom_trie_wake_node(child);
+}
+
+static void phantom_trie_wake_node(struct phantom_trie_node *node)
+{
+	phantom_trie_drain_waiters(node);
+	phantom_trie_wake_children(node);
+}
+
+/*
+ * wlink just became a directory symlink pointing at wtarget (whether
+ * converted from a phantom, or created that way outright): graft
+ * anything registered through wlink's own path onto wtarget's resolved
+ * path (see phantom_trie_graft_children()), then wake both.
+ */
+static void phantom_symlink_directory_resolved(const wchar_t *wtarget,
+						const wchar_t *wlink)
+{
+	char *own_key = phantom_symlink_canonicalize(wlink);
+	char *real_key = phantom_symlink_target_key(wtarget, wlink);
+	struct phantom_trie_node *n, *real_node;
+
+	EnterCriticalSection(&phantom_symlinks_cs);
+	n = own_key ? phantom_trie_walk(own_key, 0) : NULL;
+	real_node = real_key ? phantom_trie_walk(real_key, 1) : NULL;
+	if (n && real_node && real_node != n)
+		phantom_trie_graft_children(real_node, n);
+	if (n)
+		phantom_trie_wake_node(n);
+	if (real_node)
+		phantom_trie_wake_node(real_node);
+	LeaveCriticalSection(&phantom_symlinks_cs);
+
+	free(own_key);
+	free(real_key);
+}
+
+/* target_key: as returned by phantom_symlink_canonicalize(); not freed here */
+static void process_phantom_symlinks_under(const char *target_key)
+{
+	struct phantom_trie_node *node;
+
+	if (!target_key)
+		return;
+
+	EnterCriticalSection(&phantom_symlinks_cs);
+	node = phantom_trie_walk(target_key, 0);
+	if (node)
+		phantom_trie_wake_node(node);
 	LeaveCriticalSection(&phantom_symlinks_cs);
 }
 
@@ -494,9 +700,11 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink)
 	/* convert to directory symlink if target exists */
 	switch (process_phantom_symlink(wtarget, wlink)) {
 	case PHANTOM_SYMLINK_RETRY: {
-		/* if target doesn't exist, add to phantom symlinks list */
+		/* if target doesn't exist, add to phantom symlinks trie */
 		wchar_t wfullpath[MAX_LONG_PATH];
 		struct phantom_symlink_info *psi;
+		struct phantom_trie_node *node;
+		char *target_key;
 
 		/* convert to absolute path to be independent of cwd */
 		len = GetFullPathNameW(wlink, MAX_LONG_PATH, wfullpath, NULL);
@@ -505,6 +713,10 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink)
 			return -1;
 		}
 
+		target_key = phantom_symlink_target_key(wtarget, wlink);
+		if (!target_key)
+			break;
+
 		/* over-allocate and fill phantom_symlink_info structure */
 		psi = xmalloc(sizeof(struct phantom_symlink_info) +
 			      sizeof(wchar_t) * (len + wcslen(wtarget) + 2));
@@ -514,14 +726,20 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink)
 		wcscpy(psi->wtarget, wtarget);
 
 		EnterCriticalSection(&phantom_symlinks_cs);
-		psi->next = phantom_symlinks;
-		phantom_symlinks = psi;
+		node = phantom_trie_walk(target_key, 1);
+		if (node) {
+			psi->next = node->waiters;
+			node->waiters = psi;
+		} else {
+			free(psi);
+		}
 		LeaveCriticalSection(&phantom_symlinks_cs);
+		free(target_key);
 		break;
 	}
 	case PHANTOM_SYMLINK_DIRECTORY:
-		/* if we created a dir symlink, process other phantom symlinks */
-		process_phantom_symlinks();
+		/* if we created a dir symlink, wake others waiting on it */
+		phantom_symlink_directory_resolved(wtarget, wlink);
 		break;
 	default:
 		break;
@@ -758,8 +976,11 @@ int mingw_mkdir(const char *path, int mode UNUSED)
 		return -1;
 
 	ret = _wmkdir(wpath);
-	if (!ret)
-		process_phantom_symlinks();
+	if (!ret) {
+		char *created = phantom_symlink_canonicalize(wpath);
+		process_phantom_symlinks_under(created);
+		free(created);
+	}
 	if (!ret && needs_hiding(path))
 		return set_hidden_flag(wpath, 1);
 	return ret;
@@ -3495,7 +3716,7 @@ int mingw_create_symlink(struct index_state *index, const char *target, const ch
 			break;
 		/* There may be dangling phantom symlinks that point at this
 		 * one, which should now morph into directory symlinks. */
-		process_phantom_symlinks();
+		phantom_symlink_directory_resolved(wtarget, wlink);
 		return 0;
 	default:
 		BUG("unhandled symlink type");

t/t2041-checkout-symlink-through-symlink.sh

@@ -0,0 +1,56 @@
+#!/bin/sh
+
+test_description='checkout a symlink nested through another symlink on Windows
+
+A phantom symlink may target a path that goes through another
+symlink. Ensures that such a symlink is still upgraded to a directory
+symlink once its real target appears, even when that target directory
+is only populated after the leading symlink has already resolved.'
+
+# Tell MSYS to create native symlinks. Without this flag test-lib's
+# prerequisite detection for SYMLINKS doesn't detect the right thing.
+MSYS=winsymlinks:nativestrict && export MSYS
+
+. ./test-lib.sh
+
+if ! test_have_prereq MINGW,SYMLINKS
+then
+	skip_all='skipping $0: MinGW-only test, which requires symlink support.'
+	test_done
+fi
+
+# Adds a symlink to the index without clobbering the work tree.
+cache_symlink () {
+	sha=$(printf '%s' "$1" | git hash-object --stdin -w) &&
+	git update-index --add --cacheinfo 120000,$sha,"$2"
+}
+
+# Adds a regular file to the index without clobbering the work tree.
+cache_file () {
+	sha=$(printf '%s' "$1" | git hash-object --stdin -w) &&
+	git update-index --add --cacheinfo 100644,$sha,"$2"
+}
+
+test_expect_success 'symlink nested through another symlink resolves' '
+	test_when_finished "rm -rf chained" &&
+	mkdir chained &&
+	(
+		cd chained &&
+		git init -q &&
+
+		cache_symlink realdir leading &&
+		cache_symlink leading/sub nested &&
+		cache_file content realdir/sub/file &&
+
+		git checkout -- . &&
+
+		# "cmd.exe /c dir" marks directory symlinks as <SYMLINKD>
+		# and file symlinks as <SYMLINK>; unlike opening a path
+		# through the symlink, this distinguishes the two even
+		# though both resolve identically for plain reads.
+		cmd.exe //c dir . >dir-listing &&
+		grep "SYMLINKD.*nested" dir-listing
+	)
+'
+
+test_done