Make directory-only wildcards (cd completions) faster on slow filesystems by keeping dir information

[faho]

Description

This uses wreaddir_resolving, which tries to use the dirent d_type
field if it exists. In that way, it can skip the stat to determine
if the given file is a directory.

This allows cd completions to skip stat in most cases:

strace -Ce newfstatat fish --no-config -c 'complete -C"cd /tmp/completion_test/"' >/dev/null

prints before:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
100,00    0,002627           2      1033         4 newfstatat

after:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
100,00    0,000054           1        31         3 newfstatat

for a directory with 1000 subdirectories.

(just fish --no-config -c exit does 26 newfstatat)

This should improve the situation with slow filesystems like fuse or
network fsen.

In case we have no d_type, we use stat, which would yield about the
same results.

The worst case is that we need directories and descriptions or the
"executable" flag (which we don't currently check for cd, if I read
this right?). I don't think that case exists or is all that common?

TODOs:

• Changes to fish usage are reflected in user documentation/manpages.
• Tests have been added for regressions fixed
• User-visible changes noted in CHANGELOG.rst

[ridiculousfish]

This is great! Nice idea and perf improvement.

Curious why this is disabled under WSL?

One way to take this even further: the is_dir value is reliable, so if we have directories_only && !is_dir then we can return false immediately. This would require a three-way value (is dir, not dir, unknown) but is probably worth it since it would save additional stat calls for files which are not directories.

edit alternatively since if we have directories_only we always have a known is_dir, we only need an ordinary boolean here; we would just leave a comment about when the value is reliable.

[faho]

the is_dir value is reliable,

unfortunately it's not in all cases. i tried throwing non-dirs out early and it ran afoul of one of the logical PWD tests.

we would have to absolutize that more eagerly for this to work, and I'm not sure that's worth it.

In fact I'm not convinced there isn't already a logical $PWD bug here ("what if ../foo is a directory physically but isn't logically"). I'll have to see if I can find it.

Curious why this is disabled under WSL?

There's a check later for it, and the early-out checks simply mimic things we do anything special for later.

Edit: This is the check I'm talking about:

fish-shell/src/wildcard.cpp

Lines 477 to 480 in 3d8f98c

	if (is_windows_subsystem_for_linux() &&
	string_suffixes_string_case_insensitive(L".dll", filename)) {
	return false;
	}

This makes it so globs for completions (this is only called if we're completing) never match ".dll". That's probably too much - it should be gated by looking for executables. If that's in, we can reuse this fast path on WSL.

[faho]

Ah, got it. Two issues:

1. expand_last_segment passes an empty base_dir to wreaddir_resolving, which makes it use "/" as the base dir
2. it doesn't normalize the actual path.

fstatat won't work here because we need the normalized path for logical $PWD.

We could use path_normalize_for_cd, but that is too ornery with what it accepts - leading and trailing slashes, and if you get it wrong it assert()s out.

[faho]

Okay, benchmarks:

In a directory with 1000 directories and 1000 files, running complete -C'cd ' yields:

version	number of syscalls	time	relative
skipping early	2215	10.3ms	1.0x
not skipping early	3215	13.8ms	1.35x
lots of stat	4213	16.2ms	1.58x

The syscall count for the skipping early version does a total of 5 "getdents64", 31 "newfstatat" and 18 "openat" calls. That's 5 newfstatat and 2 openat more than a pure fish --no-config -c exit - including function files like cd.fish or source.fish.

Of the 2215 syscalls, 2000 are write() - we might want to have a look at that because it "write()"s the newline separately, which seems wasteful?