Skip to content

Commit b50d050

Browse files
mingw: avoid O(n*m) rescans when resolving phantom symlinks
On Windows, a symlink must be created as either a "file symlink" or a "directory symlink", and Git can't always tell which until the target appears during checkout; until then it's tracked as "phantom" and rechecked. Phantom symlinks lived in one global list, fully rescanned by process_phantom_symlinks() on every mkdir() (and on every symlink that turns out to point at a directory), making checkout O(n*m) in outstanding phantom symlinks times directories created. Reported in #4059 for a git-annex repo where nearly all symlinks dangle permanently. On a 343-symlink, 37,016-directory fixture, checkout takes 330s and calls process_phantom_symlink() 12,693,504 times -- about 343 * 37016. Give process_phantom_symlinks() the path that was just created (a new directory, or a symlink that turned out to point at one) as its parameter, and index phantom symlinks in a hashmap keyed by their (canonicalized, absolute) target path. Waking a path then retries only the phantom symlinks whose target is exactly that path -- an O(1) hashmap lookup -- instead of walking the whole list. Same fixture: 54.9s and 343 calls, one per symlink, no wasted rescans. Also fixes a pre-existing bug found while testing: wlink and wtarget are interior pointers into the same allocation as the phantom_symlink_info struct, not separate allocations, so they must not be free()'d individually. Known limitation: if a phantom symlink's target passes through another symlink as an intermediate component, it is only woken when that intermediate symlink itself resolves, not when the real directory it points at is populated later. This is unchanged from before this patch and expected to be rare in practice. Signed-off-by: Chris Harris <chris.harris@discordapp.com>
1 parent 734639a commit b50d050

1 file changed

Lines changed: 127 additions & 39 deletions

File tree

compat/mingw.c

Lines changed: 127 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "dir.h"
1010
#include "environment.h"
1111
#include "gettext.h"
12+
#include "hashmap.h"
1213
#include "repository.h"
1314
#include "run-command.h"
1415
#include "strbuf.h"
@@ -448,43 +449,117 @@ process_phantom_symlink(const wchar_t *wtarget, const wchar_t *wlink)
448449
return PHANTOM_SYMLINK_RETRY;
449450
}
450451

451-
/* keep track of newly created symlinks to non-existing targets */
452+
/*
453+
* Newly created symlinks to non-existing targets are indexed by a
454+
* hashmap keyed by their (canonicalized, absolute) target path, so
455+
* that mkdir() creating that path -- or a symlink at that path turning
456+
* out to be a directory symlink -- wakes only the entries actually
457+
* waiting on it, instead of re-probing every phantom symlink created
458+
* so far.
459+
*/
452460
struct phantom_symlink_info {
453-
struct phantom_symlink_info *next;
454461
wchar_t *wlink;
462+
};
463+
464+
struct phantom_symlink_target {
465+
struct hashmap_entry ent;
455466
wchar_t *wtarget;
467+
size_t nr, alloc;
468+
struct phantom_symlink_info *items;
469+
char target[FLEX_ARRAY];
456470
};
457471

458-
static struct phantom_symlink_info *phantom_symlinks = NULL;
472+
static int phantom_symlink_target_cmp(const void *cmp_data UNUSED,
473+
const struct hashmap_entry *eptr,
474+
const struct hashmap_entry *entry_or_key,
475+
const void *keydata)
476+
{
477+
const struct phantom_symlink_target *e =
478+
container_of(eptr, const struct phantom_symlink_target, ent);
479+
480+
return !fspatheq(e->target, keydata ? keydata :
481+
container_of(entry_or_key, const struct phantom_symlink_target,
482+
ent)->target);
483+
}
484+
485+
static struct hashmap phantom_symlinks =
486+
HASHMAP_INIT(phantom_symlink_target_cmp, NULL);
459487
static CRITICAL_SECTION phantom_symlinks_cs;
460488

461-
static void process_phantom_symlinks(void)
489+
static wchar_t *xwcsdup(const wchar_t *s)
462490
{
463-
struct phantom_symlink_info *current, **psi;
491+
size_t size = sizeof(wchar_t) * (wcslen(s) + 1);
492+
return memcpy(xmalloc(size), s, size);
493+
}
494+
495+
/*
496+
* Returns the canonicalized, absolute UTF-8 form of wpath. If wout is
497+
* non-NULL, also fills it with the canonicalized, absolute wide form
498+
* (must have room for at least MAX_LONG_PATH wchar_t).
499+
*/
500+
static char *canonicalize_path(const wchar_t *wpath, wchar_t *wout)
501+
{
502+
wchar_t wfullpath[MAX_LONG_PATH];
503+
char utf8[MAX_LONG_PATH * 3];
504+
int len = GetFullPathNameW(wpath, ARRAY_SIZE(wfullpath), wfullpath, NULL);
505+
506+
if (!len || len >= ARRAY_SIZE(wfullpath) ||
507+
xwcstoutf(utf8, wfullpath, sizeof(utf8)) < 0)
508+
return NULL;
509+
if (wout)
510+
wcscpy(wout, wfullpath);
511+
return xstrdup(utf8);
512+
}
513+
514+
/*
515+
* Wakes every phantom symlink whose target is exactly wpath: mkdir()
516+
* creating that path, or a symlink at that path turning out to be a
517+
* directory symlink, may let them resolve.
518+
*/
519+
static void process_phantom_symlinks(const wchar_t *wpath)
520+
{
521+
char *target_key = canonicalize_path(wpath, NULL);
522+
struct hashmap_entry key;
523+
struct phantom_symlink_target *e;
524+
size_t i;
525+
526+
if (!target_key)
527+
return;
528+
464529
EnterCriticalSection(&phantom_symlinks_cs);
465-
/* process phantom symlinks list */
466-
psi = &phantom_symlinks;
467-
while ((current = *psi)) {
468-
enum phantom_symlink_result result = process_phantom_symlink(
469-
current->wtarget, current->wlink);
530+
hashmap_entry_init(&key, fspathhash(target_key));
531+
e = hashmap_get_entry_from_hash(&phantom_symlinks, key.hash, target_key,
532+
struct phantom_symlink_target, ent);
533+
534+
for (i = 0; e && i < e->nr; ) {
535+
enum phantom_symlink_result result =
536+
process_phantom_symlink(e->wtarget, e->items[i].wlink);
537+
wchar_t *wlink = e->items[i].wlink;
538+
470539
if (result == PHANTOM_SYMLINK_RETRY) {
471-
psi = &current->next;
472-
} else {
473-
/* symlink was processed, remove from list */
474-
*psi = current->next;
475-
free(current);
476-
/* if symlink was a directory, start over */
477-
if (result == PHANTOM_SYMLINK_DIRECTORY)
478-
psi = &phantom_symlinks;
540+
i++;
541+
continue;
479542
}
543+
544+
e->items[i] = e->items[--e->nr];
545+
if (result == PHANTOM_SYMLINK_DIRECTORY)
546+
process_phantom_symlinks(wlink);
547+
free(wlink);
548+
}
549+
550+
if (e && !e->nr) {
551+
hashmap_remove(&phantom_symlinks, &e->ent, target_key);
552+
free(e->wtarget);
553+
free(e->items);
554+
free(e);
480555
}
481556
LeaveCriticalSection(&phantom_symlinks_cs);
557+
558+
free(target_key);
482559
}
483560

484561
static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink)
485562
{
486-
int len;
487-
488563
/* create file symlink */
489564
if (!CreateSymbolicLinkW(wlink, wtarget, symlink_file_flags)) {
490565
errno = err_win_to_posix(GetLastError());
@@ -494,34 +569,47 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink)
494569
/* convert to directory symlink if target exists */
495570
switch (process_phantom_symlink(wtarget, wlink)) {
496571
case PHANTOM_SYMLINK_RETRY: {
497-
/* if target doesn't exist, add to phantom symlinks list */
498-
wchar_t wfullpath[MAX_LONG_PATH];
499-
struct phantom_symlink_info *psi;
572+
/* the path process_phantom_symlink() itself probes */
573+
wchar_t relative[MAX_LONG_PATH], wfulltarget[MAX_LONG_PATH];
574+
wchar_t wfulllink[MAX_LONG_PATH];
575+
const wchar_t *rel = make_relative_to(wtarget, wlink, relative,
576+
ARRAY_SIZE(relative));
577+
char *target_key = rel ? canonicalize_path(rel, wfulltarget) : NULL;
578+
struct hashmap_entry key;
579+
struct phantom_symlink_target *e;
580+
int len;
581+
582+
if (!target_key)
583+
break;
500584

501585
/* convert to absolute path to be independent of cwd */
502-
len = GetFullPathNameW(wlink, MAX_LONG_PATH, wfullpath, NULL);
503-
if (!len || len >= MAX_LONG_PATH) {
586+
len = GetFullPathNameW(wlink, ARRAY_SIZE(wfulllink), wfulllink, NULL);
587+
if (!len || len >= ARRAY_SIZE(wfulllink)) {
504588
errno = err_win_to_posix(GetLastError());
589+
free(target_key);
505590
return -1;
506591
}
507592

508-
/* over-allocate and fill phantom_symlink_info structure */
509-
psi = xmalloc(sizeof(struct phantom_symlink_info) +
510-
sizeof(wchar_t) * (len + wcslen(wtarget) + 2));
511-
psi->wlink = (wchar_t *)(psi + 1);
512-
wcscpy(psi->wlink, wfullpath);
513-
psi->wtarget = psi->wlink + len + 1;
514-
wcscpy(psi->wtarget, wtarget);
515-
516593
EnterCriticalSection(&phantom_symlinks_cs);
517-
psi->next = phantom_symlinks;
518-
phantom_symlinks = psi;
594+
hashmap_entry_init(&key, fspathhash(target_key));
595+
e = hashmap_get_entry_from_hash(&phantom_symlinks, key.hash,
596+
target_key,
597+
struct phantom_symlink_target, ent);
598+
if (!e) {
599+
FLEX_ALLOC_STR(e, target, target_key);
600+
e->wtarget = xwcsdup(wfulltarget);
601+
hashmap_entry_init(&e->ent, key.hash);
602+
hashmap_add(&phantom_symlinks, &e->ent);
603+
}
604+
ALLOC_GROW(e->items, e->nr + 1, e->alloc);
605+
e->items[e->nr++].wlink = xwcsdup(wfulllink);
519606
LeaveCriticalSection(&phantom_symlinks_cs);
607+
free(target_key);
520608
break;
521609
}
522610
case PHANTOM_SYMLINK_DIRECTORY:
523-
/* if we created a dir symlink, process other phantom symlinks */
524-
process_phantom_symlinks();
611+
/* if we created a dir symlink, wake others waiting on it */
612+
process_phantom_symlinks(wlink);
525613
break;
526614
default:
527615
break;
@@ -759,7 +847,7 @@ int mingw_mkdir(const char *path, int mode UNUSED)
759847

760848
ret = _wmkdir(wpath);
761849
if (!ret)
762-
process_phantom_symlinks();
850+
process_phantom_symlinks(wpath);
763851
if (!ret && needs_hiding(path))
764852
return set_hidden_flag(wpath, 1);
765853
return ret;
@@ -3495,7 +3583,7 @@ int mingw_create_symlink(struct index_state *index, const char *target, const ch
34953583
break;
34963584
/* There may be dangling phantom symlinks that point at this
34973585
* one, which should now morph into directory symlinks. */
3498-
process_phantom_symlinks();
3586+
process_phantom_symlinks(wlink);
34993587
return 0;
35003588
default:
35013589
BUG("unhandled symlink type");

0 commit comments

Comments
 (0)