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

> 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?