Reduce stat calls for wildcard

[faho]

Description

This attempts to reduce the number of stat calls we do for wildcard in the case of a final */ component. That * is taken as an "intermediate" segment, followed by an empty final segment. This makes it so the intermediate segment expansion knows about it and stops the stat() calls we do for the purpose of symlink cycle detection (since we do not enter any symlinks we can't add any new cycles there!).

true ~/complete_test/*/ with a complete_test directory with 10000 subdirectories is sped up by 1.60x relative to 35baa88 by the first commit.

This is on ext4 with proper d_type support, so we get all the info via readdir, and can save 10k stat calls for 10k files

This is in C++ because that's the version that's currently used. If we decide it's a good approach, I'll port it to rust, and we can easily cherry-pick it for 3.7.0.

(I also had something for ls ~/foo/<TAB>, but that didn't end up saving anything and was kinda messy so I removed it - ignore the second commit)

TODOs:

• User-visible changes noted in CHANGELOG.rst

[ridiculousfish]

This causes a visible change when the final path component is a symlink loop. You can reproduce like so, in an initially empty directory:

> ln -s . loop
> echo */*/

Before this change, the wildcard matching would fail, because we would notice that the directory had already been visited. After this change, we no longer check for a visited directory in the last intermediate segment (the second *) so it outputs loop/loop/.

I'm not sure if this behavior is better or worse: symlink loops are mainly a problem for recursive wildcards. But we should probably decide which behavior we want before making a behavior change.

Meanwhile, I think you can still get almost all the perf benefit by only using the fast path if the directory entry is not a symlink, which is returned from the d_type. This would require adding that information to dir_iter_t::entry_t.

[faho]

I'm not sure if this behavior is better or worse: symlink loops are mainly a problem for recursive wildcards. But we should probably decide which behavior we want before making a behavior change.

If I understand this correctly, loop/loop would be printed once, it would not be an infinite recursion?

I would find that acceptable in a vacuum, but I'm wary of the change.

Meanwhile, I think you can still get almost all the perf benefit by only using the fast path if the directory entry is not a symlink, which is returned from the d_type. This would require adding that information to dir_iter_t::entry_t.

That should work, because we do enter the link once - if there's a directory with "foo" and "loop", we get "foo" and "loop/foo". The one we don't get is "loop/loop".

So we really just need the information that we've seen this link already.

[faho]

If I do add the link fast path (add a "possible_link" bool that is set to false if we don't get DT_LNK or DT_UNKNOWN), it fails this check:

# But symlink loops only get explored once.
mkdir -p dir1/child2/grandchild1
touch dir1/child2/grandchild1/differentfile
ln -s ../../child2/grandchild1 dir1/child2/grandchild1/link2
echo **/differentfile
# CHECK: dir1/child2/grandchild1/differentfile

Because that expects the path via the link to not be counted because it knows we got the file. This would require us to notice we already visited "differentfile". And because the first path to that does not go via any link, and the link starts at "dir1/", we would need to stat() dir1/ without it being a link.

---

Honestly I am not entirely sure I understand the behavior here. I get that we want to stop symlink loops.

But it only really stops for the */*/ case when the first * resolves to a link - where we match a directory under the link. It doesn't stop */* or ** from printing the same effective files twice, once via the physical path and once through the looping link. Or rather once for each looping symlink - if you do ln -s . loop; ln -s . loop2, you'll get both loop/foo and loop2/foo.

It is fundamentally weird to me that */* matches a directory "loop/foo", but */*/ does not - it is a directory. Basically I think of */ as the last element with a "/" to make it directories only, while the code here treats it as an "intermediate" element with an empty last element.

Anyway, given the above I do not believe this is possible without a behavior change.

[faho]

Okay, this now only skips the file_id_t if it isn't a link and it's the last element.

That means */ (and any glob ending in */ is no longer measurably slower than * (if d_type is used), and I can't find a case where it fails - */*/ still doesn't match. It skips 1 stat per-directory.

It's unfortunately a pretty narrow case, and I still don't understand why the loop detection is that way.

But for that narrow and reasonably common case it's a sweet and reasonably simple fix.

[ridiculousfish]

Nice fix! I agree the current behavior is a little confusing but this is a great optimization in the mean time. One thing to fix.

[ridiculousfish]

entry_.possible_link_ should be set to true or false in every path in this function, as we re-use the entry_ variable to save allocations.