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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2339 bytes --]

On Wed, Jan 10, 2024 at 12:07:52PM -0800, Junio C Hamano wrote:
> 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?

It likely does not matter much. This function only gets called when we
have previously determined the stack to be out of date already. So
unless we update the validity cache, we know that the next validity
check would continue to treat the stack as outdated.

That being said I think it's good hygiene to clear the validity cache
regardless of that.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]