Skip to content

Commit e89501c

Browse files
committed
files-backend.c: avoid stat in 'loose_fill_ref_dir'
Modify the 'readdir' loop in 'loose_fill_ref_dir' to, rather than 'stat' a file to determine whether it is a directory or not, use 'get_dtype'. Currently, the loop uses 'stat' to determine whether each dirent is a directory itself or not in order to construct the appropriate ref cache entry. If 'stat' fails (returning a negative value), the dirent is silently skipped; otherwise, 'S_ISDIR(st.st_mode)' is used to check whether the entry is a directory. On platforms that include an entry's d_type in in the 'dirent' struct, this extra 'stat' check is redundant. We can use the 'get_dtype' method to extract this information on platforms that support it (i.e. where NO_D_TYPE_IN_DIRENT is unset), and derive it with 'stat' on platforms that don't. Because 'stat' is an expensive call, this confers a modest-but-noticeable performance improvement when iterating over large numbers of refs (approximately 20% speedup in 'git for-each-ref' in a 30k ref repo). Unlike other existing usage of 'get_dtype', the 'follow_symlinks' arg is set to 1 to replicate the existing handling of symlink dirents. This unfortunately requires calling 'stat' on the associated entry regardless of platform, but symlinks in the loose ref store are highly unlikely since they'd need to be created manually by a user. Note that this patch also changes the condition for skipping creation of a ref entry from "when 'stat' fails" to "when the d_type is anything other than DT_REG or DT_DIR". If a dirent's d_type is DT_UNKNOWN (either because the platform doesn't support d_type in dirents or some other reason) or DT_LNK, 'get_dtype' will try to derive the underlying type with 'stat'. If the 'stat' fails, the d_type will remain 'DT_UNKNOWN' and dirent will be skipped. However, it will also be skipped if it is any other valid d_type (e.g. DT_FIFO for named pipes, DT_LNK for a nested symlink). Git does not handle these properly anyway, so we can safely constrain accepted types to directories and regular files. Signed-off-by: Victoria Dye <vdye@github.com>
1 parent 295ca94 commit e89501c

File tree

1 file changed

+5
-9
lines changed

1 file changed

+5
-9
lines changed

refs/files-backend.c

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -246,10 +246,8 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
246246
int dirnamelen = strlen(dirname);
247247
struct strbuf refname;
248248
struct strbuf path = STRBUF_INIT;
249-
size_t path_baselen;
250249

251250
files_ref_path(refs, &path, dirname);
252-
path_baselen = path.len;
253251

254252
d = opendir(path.buf);
255253
if (!d) {
@@ -262,23 +260,22 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
262260

263261
while ((de = readdir(d)) != NULL) {
264262
struct object_id oid;
265-
struct stat st;
266263
int flag;
264+
unsigned char dtype;
267265

268266
if (de->d_name[0] == '.')
269267
continue;
270268
if (ends_with(de->d_name, ".lock"))
271269
continue;
272270
strbuf_addstr(&refname, de->d_name);
273-
strbuf_addstr(&path, de->d_name);
274-
if (stat(path.buf, &st) < 0) {
275-
; /* silently ignore */
276-
} else if (S_ISDIR(st.st_mode)) {
271+
272+
dtype = get_dtype(de, &path, 1);
273+
if (dtype == DT_DIR) {
277274
strbuf_addch(&refname, '/');
278275
add_entry_to_dir(dir,
279276
create_dir_entry(dir->cache, refname.buf,
280277
refname.len));
281-
} else {
278+
} else if (dtype == DT_REG) {
282279
if (!refs_resolve_ref_unsafe(&refs->base,
283280
refname.buf,
284281
RESOLVE_REF_READING,
@@ -308,7 +305,6 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
308305
create_ref_entry(refname.buf, &oid, flag));
309306
}
310307
strbuf_setlen(&refname, dirnamelen);
311-
strbuf_setlen(&path, path_baselen);
312308
}
313309
strbuf_release(&refname);
314310
strbuf_release(&path);

0 commit comments

Comments
 (0)