On Tue, Oct 5, 2010 at 4:53 PM, Allan McRae <allan@archlinux.org> wrote:
On 06/10/10 02:49, Dan McGee wrote:
On Linux and OS X, we can determine if an entry obtained through a readdir() call is a directory without also having to stat it. This can save a significant number of syscalls; it does make the getdents() call more expensive but cuts out a lot of stat() calls.
Before: $ strace -c pacman -Ss pacman % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 82.95 0.016153 1 21733 read 4.21 0.000819 0 11056 23 open 3.96 0.000771 0 17602 1 access 1.84 0.000358 0 11026 fstat 1.72 0.000334 0 11033 close 1.54 0.000299 0 6608 3 stat 0.00 0.000000 0 20 getdents ------ ----------- ----------- --------- --------- ---------------- 100.00 0.019473 101271 28 total
After: $ strace -c ./src/pacman/.libs/lt-pacman -Ss pacman % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 30.11 0.001503 75 20 getdents 14.81 0.000739 0 21733 read 13.91 0.000694 0 17602 1 access 13.32 0.000665 0 11072 39 open 6.61 0.000330 0 11026 fstat 6.03 0.000301 0 11033 close 0.00 0.000000 0 9 6 stat ------ ----------- ----------- --------- --------- ---------------- 100.00 0.004991 94686 47 total
Obviously the numbers will show some variation, but it never seems to be slower so this should be a win overall.
Signed-off-by: Dan McGee<dan@archlinux.org> ---
List,
My take on a rather stale patch in my inbox. I added the is_dir() static function so we can still run on platforms not supporting this shortcut, and I also wasn't sure why the access() call was removed in the orignal patch (commit messages explaining changes, anyone?). Let me know what you think.
This has been sitting in my flagged emails folder for a while too...
I always figured the access part was being over heavy-handed in removal of checks at the time. However, we now have check that fopen on the "desc" etc files is successful (see right below it) so perhaps it is indeed now excessive to check the directory first?
I'm partial to the fail-early, fail-often mantra, but someone else is welcome to submit a patch with sufficient explanation as to why it isn't necessary if we still feel it is overkill. The dir scanning stuff was definitely the bulk of the reduction in time here; I'm not so certain the extra 7ms are going to be noticed for the access() overhead. -Dan