Re: [PATCH 3/4] reftable/stack: use stat info to avoid re-reading stack list

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> We can do better and use the same stat(3P)-based mechanism that the
> "packed" backend uses. Instead of reading the file, we will only open
> the file descriptor, fstat(3P) it, and then compare the info against the
> cached value from the last time we have updated the stack. This should
> always work alright because "tables.list" is updated atomically via a
> rename, so even if the ctime or mtime wasn't granular enough to identify
> a change, at least the inode number should have changed.

Or the file size.  Let's keep in mind that many users get useless
inum from their filesystem X-<.

>   Summary
>     update-ref: create many refs (refcount = 1, revision = HEAD~) ran
>       1.01 ± 0.09 times faster than update-ref: create many refs (refcount = 1, revision = HEAD)
>       2.72 ± 0.11 times faster than update-ref: create many refs (refcount = 100, revision = HEAD)
>       3.42 ± 0.13 times faster than update-ref: create many refs (refcount = 100, revision = HEAD~)
>     163.59 ± 5.62 times faster than update-ref: create many refs (refcount = 10000, revision = HEAD)
>     233.91 ± 7.92 times faster than update-ref: create many refs (refcount = 10000, revision = HEAD~)
> ---

Nice.

> @@ -374,6 +375,8 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
>  		sleep_millisec(delay);
>  	}
>  
> +	stat_validity_update(&st->list_validity, fd);
> +
>  out:
>  	if (fd >= 0)
>  		close(fd);

The stat_validity_update() does not happen in the error codepath.

Should we be clearing the validity of the list when somebody jumps
to "out:" due to an error?  Or by the time this function gets
called, the caller would already have cleared the validity and an
error that jumps to "out:" keeps the list invalid?

Other than the missing sign-off, the change looks very straight-forward.