buggrep - Bugs: bug #30868, Take advantage of dirent.d_type to...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #30868: Take advantage of dirent.d_type to save a stat()

Submitted by:  Nick Welch <mackstann>
Submitted on:  Tue 24 Aug 2010 10:22:40 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: None
Open/Closed: Closed

Thu 15 Mar 2012 10:00:12 PM UTC, comment #3:

I fixed this, I think, in quite a different way, with this patch which was recently committed:

http://git.savannah.gnu.org/cgit/grep.git/commit/?id=c6e3ea61d9f08aa0128a0eb13d31a2fbad376f99

So I'm closing the bug report as 'fixed'; if there's a problem with the solution, please let me know and we can reopen the bug.

Paul Eggert <eggert>
Project Member
Fri 27 Aug 2010 03:20:38 AM UTC, comment #2:

Sorry, that patch was incomplete. Here is the full patch.

(file #21320)

Nick Welch <mackstann>
Thu 26 Aug 2010 07:06:32 AM UTC, comment #1:

I have attached a proof of concept patch that implements this. I'm sure it breaks some coding conventions and could use some adjustment, but it does work and passes the test suite.

Watching the action through strace, it now avoids calling stat() on excluded files at all. (It's worth noting that this optimization only comes into play when excluding/including files/dirs.)

The conditions under which this is a significant speedup appear to be fairly rare. The cost of stat()ing files seems be a low percentage of the overall runtime. Even when reading through file contents is completely avoided by doing something like --include '*.asdfasdfasdf', the speedup is only around 10% for me (searching through all of /usr recursively). Under normal conditions the speed improvement seems to be lost in noise.

However, these measurements could vary wildly between OS, filesystem, hardware, etc., for all I know. I tested the latest grep from git, on Ubuntu 10.04, on a mechanical hard drive running ext4. I also made sure to drop all disk caches before each test by echoing 3 to /proc/sys/vm/drop_caches.

I'm also confused about how the savedir code is maintained. The version in gnulib has the same interface but is significantly different from the version in grep. The gnulib version doesn't even attempt to check file type.

(file #21311)

Nick Welch <mackstann>
Tue 24 Aug 2010 10:22:40 PM UTC, original submission:

In lib/savedir.c, in isdir1() (or the code that calls it), the d_type member of the dirent struct can be checked instead of doing a comparatively expensive isdir() (which calls stat()).

This d_type member is not supported by all systems or filesystems. The block of code referencing d_type should be wrapped with an #ifdef _DIRENT_HAVE_D_TYPE. The value of d_type will be DT_UNKNOWN if the filesystem does not support d_type.

See here for more details: http://manpages.courier-mta.org/htmlman3/readdir.3.html

Nick Welch <mackstann>

 

Attached Files
file #21320:  0001-Check-dirent.d_type-to-avoid-many-stat-s.patch added by mackstann (2KiB - text/x-patch - proof of concept patch, second version)
file #21311:  0001-Check-dirent.d_type-to-avoid-many-stat-s.patch added by mackstann (1KiB - text/x-patch - proof of concept patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by eggert (Posted a comment)
  • -unavailable- added by mackstann (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 15 Mar 2012 10:00:12 PM UTCeggertStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Fri 27 Aug 2010 03:20:38 AM UTCmackstannAttached File-=>Added 0001-Check-dirent.d_type-to-avoid-many-stat-s.patch, #21320
    Thu 26 Aug 2010 07:06:32 AM UTCmackstannAttached File-=>Added 0001-Check-dirent.d_type-to-avoid-many-stat-s.patch, #21311

    Back to the top


    Powered by Savane 3.1-cleanup1