[pacman-dev] [PATCH] Avoid stat call to determine is_directory if possible

[Dan McGee]

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 at 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.

-Dan

-Dan

 lib/libalpm/be_files.c |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c
index 4432171..0f055e0 100644
--- a/lib/libalpm/be_files.c
+++ b/lib/libalpm/be_files.c
@@ -108,13 +108,28 @@ static int dirlist_from_tar(const char *archive, alpm_list_t **dirlist)
 	return(0);
 }
 
+static int is_dir(const char *path, struct dirent *entry)
+{
+#ifdef DT_DIR
+	return(entry->d_type == DT_DIR);
+#else
+	char buffer[PATH_MAX];
+	snprintf(buffer, PATH_MAX, "%s/%s", path, entry->d_name);
+
+	struct stat sbuf;
+	if (!stat(buffer, &sbuf)) {
+		return(S_ISDIR(sbuf.st_mode));
+	}
+
+	return(0);
+#endif
+}
+
 /* create list of directories in db */
 static int dirlist_from_fs(const char *syncdbpath, alpm_list_t **dirlist)
 {
 	DIR *dbdir;
 	struct dirent *ent = NULL;
-	struct stat sbuf;
-	char path[PATH_MAX];
 
 	dbdir = opendir(syncdbpath);
 	if (dbdir != NULL) {
@@ -127,9 +142,7 @@ static int dirlist_from_fs(const char *syncdbpath, alpm_list_t **dirlist)
 				continue;
 			}
 
-			/* stat the entry, make sure it's a directory */
-			snprintf(path, PATH_MAX, "%s%s", syncdbpath, name);
-			if(stat(path, &sbuf) != 0 || !S_ISDIR(sbuf.st_mode)) {
+			if(!is_dir(syncdbpath, ent)) {
 				continue;
 			}
 
@@ -353,8 +366,6 @@ int _alpm_db_populate(pmdb_t *db)
 {
 	int count = 0;
 	struct dirent *ent = NULL;
-	struct stat sbuf;
-	char path[PATH_MAX];
 	const char *dbpath;
 	DIR *dbdir;
 
@@ -374,9 +385,7 @@ int _alpm_db_populate(pmdb_t *db)
 		if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) {
 			continue;
 		}
-		/* stat the entry, make sure it's a directory */
-		snprintf(path, PATH_MAX, "%s%s", dbpath, name);
-		if(stat(path, &sbuf) != 0 || !S_ISDIR(sbuf.st_mode)) {
+		if(!is_dir(dbpath, ent)) {
 			continue;
 		}
 
-- 
1.7.3.1