On 05/23/2015 01:51 AM, Jeff King wrote:
> The stat_validity code was originally written to avoid
> re-reading the packed-refs file when it has not changed. It
> makes sure that the file continues to match S_ISREG() when
> we check it.
>
> However, we can use the same concept on a directory to see
> whether it has been modified. Even though we still have to
> touch the filesystem to do the stat, this can provide a
> speedup even over opendir/readdir/closedir, especially on
> high-latency filesystems like NFS.
>
> This patch adds a "mode" field to stat_validity, which lets
> us check that the mode has stayed the same (rather than
> explicitly checking that it is a regular file).
I don't have any insight about whether mtimes are reliable change
indicators for directories.
But if you make this change, you are changing the contract of the
stat_validity functions:
* Have you verified that no callers rely on the old stat_validity's
check that the file is a regular file?
* The docstrings in cache.h need to be updated.
> As a bonus cleanup, we can stop allocating the embedded
> "stat_data" on the heap. Prior to this patch, we needed to
> represent the case where the file did not exist, and we used
> "sv->sd == NULL" for that. Now we can simply check that
> "sv->mode" is 0.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> cache.h | 3 ++-
> read-cache.c | 16 ++++++----------
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index c97d807..2941e7e 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1660,7 +1660,8 @@ int sane_execvp(const char *file, char *const argv[]);
> * for the index.
> */
> struct stat_validity {
> - struct stat_data *sd;
> + struct stat_data sd;
> + unsigned mode;
> };
Could the mode be stored directly in stat_data?
By comparing modes, you are not only comparing file type (S_ISREG vs
S_ISDIR etc.) but also all of the file permissions. That seems OK with
me but you might want to document that fact. Plus, I don't know whether
POSIX allows additional, implementation-defined information to be stored
in st_mode. If so, you would be comparing that, too.
> [...]
Otherwise, looks OK.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html