Speed up executable command completions#7153
Conversation
src/wildcard.cpp
Outdated
| std::vector<gid_t> groups; | ||
| while (true) { | ||
| int ngroups = getgroups(0, nullptr); | ||
| assert(ngroups > 0 && "getgroups(0, nullptr) returned 0 group memberships!"); |
There was a problem hiding this comment.
I think technically you can have no supplementary groups? The spec is not very clear.
There was a problem hiding this comment.
I forgot that the effective group/primary group is not guaranteed to be included and was presuming that would be the at least one response. You're right, we should probably remove that hard assert.
|
How much faster do you think this makes things? |
|
Completions are around 25% faster (on Linux 5.3 with pti enabled), taking 34% less system cpu time. I've been leery of syscall-heavy code because it just seems to get slower and slower these days. (Benchmark was after rebasing against master.) Note that this is on an NVMe and after priming the cache. The syscalls involve inode metadata access, so on spinning rust you can expect much bigger improvements. |
| return COMPLETE_SOCKET_DESC; | ||
| } else if (S_ISDIR(buf.st_mode)) { | ||
| return COMPLETE_DIRECTORY_DESC; | ||
| } else if (buf.st_mode & (S_IXUSR | S_IXGRP | S_IXGRP) && waccess(filename, X_OK) == 0) { |
There was a problem hiding this comment.
I'm pretty sure this is a bug and was supposed to be S_IXUSR | S_IXGRP | S_IXOTH
|
On Linux and BSD it is not possible to effectively* apply separate permissions to a symlink than the destination it points to. Does anyone know if there are any platforms that do not follow this rule (@krobelus @zanchey)? e.g. > touch ./executable
> chmod 777 ./executable
> ln -s ./executable ./symlink
> chmod 077 ./symlink
> ls -alhtr ./executable ./symlink
----rwxrwx 1 mqudsi mqudsi 0 Jun 28 16:04 ./executable
lrwxrwxrwx 1 mqudsi mqudsi 12 Jun 28 16:04 ./symlink -> ./executablefrom * the symlink always has full permissions so the target is always the determinate |
It's not surprising that this never came up, as I cannot imagine a more useless chmod value. Perhaps in the context of nologin or something? *shrug*
|
At least on FreeBSD, the symbolic link can have a different mode than the target file - the |
|
Oh, sorry - effectively - you are right |
This brings down the number of syscalls per potential completion result from three or four to just one.
This brings down the number of syscalls per potential completion result from three or four to just one.
It passes the tests and I'm comfortable with how it works on Linux and BSD, but putting this up here before merging in case anyone spots something non-portable or can think of any edge cases that aren't addressed.