* [PATCH] Use access instead of stat in hashdir
@ 2012-02-06 13:13 Raghavendra D Prabhu
2012-02-06 16:20 ` Bart Schaefer
0 siblings, 1 reply; 5+ messages in thread
From: Raghavendra D Prabhu @ 2012-02-06 13:13 UTC (permalink / raw)
To: Zsh workers
[-- Attachment #1: Type: text/plain, Size: 839 bytes --]
Hi,
I found that using access instead of two stat calls results in
faster rehash when it is done. I came across this when I noticed
too many stat calls while 'strace -c'
diff --git a/Src/hashtable.c b/Src/hashtable.c
index 775b6a2..43d2e2e 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -664,8 +664,7 @@ hashdir(char **dirp)
* executable plain files.
*/
if (unset(HASHEXECUTABLESONLY) ||
- (stat(pathbuf, &statbuf) == 0 &&
- S_ISREG(statbuf.st_mode) && (statbuf.st_mode & S_IXUGO)))
+ !access(pathbuf,X_OK))
add = 1;
}
if (add) {
Regards,
--
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Use access instead of stat in hashdir
2012-02-06 13:13 [PATCH] Use access instead of stat in hashdir Raghavendra D Prabhu
@ 2012-02-06 16:20 ` Bart Schaefer
2012-02-06 16:52 ` Bart Schaefer
2012-02-07 22:46 ` Raghavendra D Prabhu
0 siblings, 2 replies; 5+ messages in thread
From: Bart Schaefer @ 2012-02-06 16:20 UTC (permalink / raw)
To: Raghavendra D Prabhu, Zsh workers
Thanks for the suggestion, but ...
On Feb 6, 6:43pm, Raghavendra D Prabhu wrote:
}
} I found that using access instead of two stat calls
There's only one stat() call in the lines that you changed ...?
} results in
} faster rehash when it is done. I came across this when I noticed
} too many stat calls while 'strace -c'
It also results in treating non-regular files as candidates for being
in the hash table, unless there's something about access() that is
implicitly performing the S_ISREG() test.
So you've broken the correctness of the HASH_EXECUTABLES_ONLY option.
Why not just leave it unset instead? That's why it's an option.
} if (unset(HASHEXECUTABLESONLY) ||
} - (stat(pathbuf, &statbuf) == 0 &&
} - S_ISREG(statbuf.st_mode) && (statbuf.st_mode & S_IXUGO)))
} + !access(pathbuf,X_OK))
} add = 1;
} }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Use access instead of stat in hashdir
2012-02-06 16:20 ` Bart Schaefer
@ 2012-02-06 16:52 ` Bart Schaefer
2012-02-07 22:46 ` Raghavendra D Prabhu
1 sibling, 0 replies; 5+ messages in thread
From: Bart Schaefer @ 2012-02-06 16:52 UTC (permalink / raw)
To: Zsh workers
On Feb 6, 8:20am, Bart Schaefer wrote:
>
> On Feb 6, 6:43pm, Raghavendra D Prabhu wrote:
> }
> } I found that using access instead of two stat calls results in
> } faster rehash when it is done. I came across this when I noticed
> } too many stat calls while 'strace -c'
>
> } if (unset(HASHEXECUTABLESONLY) ||
> } - (stat(pathbuf, &statbuf) == 0 &&
> } - S_ISREG(statbuf.st_mode) && (statbuf.st_mode & S_IXUGO)))
> } + !access(pathbuf,X_OK))
> } add = 1;
> } }
In Src/exec.c the function iscom():
int
iscom(char *s)
{
struct stat statbuf;
char *us = unmeta(s);
return (access(us, X_OK) == 0 && stat(us, &statbuf) >= 0 &&
S_ISREG(statbuf.st_mode));
}
So let's see if we can get some speed out of this:
Index: Src/hashtable.c
===================================================================
diff -c -r1.20 Src/hashtable.c
--- hashtable.c 1 Jun 2011 06:40:00 -0000 1.20
+++ hashtable.c 6 Feb 2012 16:49:24 -0000
@@ -663,8 +663,10 @@
* This is the same test as for the glob qualifier for
* executable plain files.
*/
- if (stat(pathbuf, &statbuf) == 0 &&
- S_ISREG(statbuf.st_mode) && (statbuf.st_mode & S_IXUGO))
+ if (unset(HASHEXECUTABLESONLY) ||
+ (access(pathbuf, X_OK) == 0 &&
+ stat(pathbuf, &statbuf) == 0 &&
+ S_ISREG(statbuf.st_mode) && (statbuf.st_mode & S_IXUGO)))
add = 1;
}
if (add) {
The S_IXUGO test may now be redundant, but hardly worth skipping.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] Use access instead of stat in hashdir
2012-02-06 16:20 ` Bart Schaefer
2012-02-06 16:52 ` Bart Schaefer
@ 2012-02-07 22:46 ` Raghavendra D Prabhu
2012-02-08 0:40 ` Bart Schaefer
1 sibling, 1 reply; 5+ messages in thread
From: Raghavendra D Prabhu @ 2012-02-07 22:46 UTC (permalink / raw)
To: Bart Schaefer; +Cc: Zsh workers
[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]
Hi,
* On Mon, Feb 06, 2012 at 08:20:08AM -0800, Bart Schaefer <schaefer@brasslantern.com> wrote:
>Thanks for the suggestion, but ...
>
>On Feb 6, 6:43pm, Raghavendra D Prabhu wrote:
>}
>} I found that using access instead of two stat calls
>
>There's only one stat() call in the lines that you changed ...?
Sorry, I meant the other checks.
>
>} results in
>} faster rehash when it is done. I came across this when I noticed
>} too many stat calls while 'strace -c'
>
>It also results in treating non-regular files as candidates for being
>in the hash table, unless there's something about access() that is
>implicitly performing the S_ISREG() test.
Need to verify this but for X_OK the file will need to be regular
right ?
>
>So you've broken the correctness of the HASH_EXECUTABLES_ONLY option.
>Why not just leave it unset instead? That's why it's an option.
>
>} if (unset(HASHEXECUTABLESONLY) ||
>} - (stat(pathbuf, &statbuf) == 0 &&
>} - S_ISREG(statbuf.st_mode) && (statbuf.st_mode & S_IXUGO)))
>} + !access(pathbuf,X_OK))
>} add = 1;
>} }
>
Yes, I recently merged the tree with mine which I was using
before HASHEXECUTABLESONLY was there, need to check on that.
Regards,
--
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] Use access instead of stat in hashdir
2012-02-07 22:46 ` Raghavendra D Prabhu
@ 2012-02-08 0:40 ` Bart Schaefer
0 siblings, 0 replies; 5+ messages in thread
From: Bart Schaefer @ 2012-02-08 0:40 UTC (permalink / raw)
To: Zsh workers
On Tue, Feb 7, 2012 at 2:46 PM, Raghavendra D Prabhu
<raghu.prabhu13@gmail.com> wrote:
> * On Mon, Feb 06, 2012 at 08:20:08AM -0800, Bart Schaefer
> <schaefer@brasslantern.com> wrote:
>> } faster rehash when it is done. I came across this when I noticed
>> } too many stat calls while 'strace -c'
>>
>> It also results in treating non-regular files as candidates for being
>> in the hash table, unless there's something about access() that is
>> implicitly performing the S_ISREG() test.
>
> Need to verify this but for X_OK the file will need to be regular right ?
DESCRIPTION
access() checks whether the process would be allowed to read, write or
test for existence of the file (or other file system object) whose name
is pathname. If pathname is a symbolic link permissions of the file
referred to by this symbolic link are tested. [...]
Only access bits are checked, not the file type or contents. [...]
If the process has appropriate privileges, an implementation may indi-
cate success for X_OK even if none of the execute file permission bits
are set.
Note "or other file system object."
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-02-08 0:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-06 13:13 [PATCH] Use access instead of stat in hashdir Raghavendra D Prabhu
2012-02-06 16:20 ` Bart Schaefer
2012-02-06 16:52 ` Bart Schaefer
2012-02-07 22:46 ` Raghavendra D Prabhu
2012-02-08 0:40 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).