On Wed, 8 Mar 2017 14:41:25 -0800, Jun Wu wrote:
> Excerpts from Simon Farnsworth's message of 2017-03-08 18:52:06 +0000:
> > > + A load function usually looks like:
> > > +
> > > + def loadfunc(state, oldhash, oldvalue):
> > > + hash = state.quickhash()
> > > + if hash == oldhash:
> > > + return oldhash, oldvalue
> > > + value = state.loadvalue()
> > > + hash = hashvalue(value)
> > > + # or, if hashvalue is expensive
> > > + hashagain = state.quickhash()
> > > + if hashagain != hash:
> > > + # invalidate the cache entry without filling a new one
> > > + hash = None
> > > + return hash, value
> > > +
> >
> > I don't like this interface - there's a lot for a load function author
> > to get right. I would prefer to see a split into a pair of functions -
> > one that gets the hash, one that gets the data - and have the cache code
> > handle the state management instead.
>
> There could be multiple ways to get hash values - a quick way to test if the
> cache can be used or not, and another way if you know the value (to avoid
> races, requires some state about "value"). For example, if we use the length
> of obsstore as the hash:
>
> def loadobsstore(repo, oldlen, oldvalue):
> newlen = repo.svfs.stat('obsstore').st_size # <- hash function 1
> if newlen == oldlen:
> return oldlen, oldvalue
> content = repo.svfs.read('obsstore')
> newvalue = parseobsstore(content)
> newlen = len(content) # <- hash function 2, no race condition
> # no unnecessary syscall of "stat"
> # not easy if hash function is separated
> return newlen, newvalue
>
> This pattern also makes sure that it's impossible to run the loading
> function alone and get into possible cache invalidation trouble.
>
> Besides, "loadfunctable" fits the registrar framework:
>
> # extension foo
> preloadtable = {}
> preload = something.preload(preloadtable)
>
> @preload('changelog')
> def changelogloader(repo, hash, value):
> ....
>
> A pair of functions won't fit the decorator pattern cleanly.
That could use @property and .setter pattern. It will make the decorator
complicated, but worth trying if it can simply the loader interface.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
