Making chg stateful

[Jun Wu]

Excerpts from Yuya Nishihara's message of 2017-02-05 13:59:32 +0900:
> Perhaps that is a matter of taste, I like the explicit one. But I heard
> Python 3 has a generator-based coroutine, so using "yield" for that purpose
> is more common in Python 3?
>
>     @preload('index')
>     def preloadindex(repo):
>         f = repo.svfs('changelog.i') # "f" can be shared naturally
>         hash = fstat(f).st_size
>         def loadindex():
>             indexdata = f.read()
>             hash = len(indexdata)
>             return parseindex(indexdata), hash

The inner "loadindex" function makes things unnecessarily more complex in my
opinion. It makes the final return type harder to read, introduces extra
indentation and a function definition.

If the goal is to get rid of "yield", it could be done in different ways.
For example,

Choice A: explicitly pass oldhash and the oldobject. We probably need
the old object anyway to allow incremental updates:

    @preload('obsstore', depend=['changelog'])
    def preloadobsstore(repo, oldhash, oldobsstore):
        obsstore = oldobsstore
        newhash = ...
        if newhash != oldhash:
             # note: this is an indentation not existed using yield
            obsstore = ...
        return newhash, obsstore

Choice B: just give the preload function access to the cache object:

    @preload(['obsstore', 'changelog'])
    def preloadmultiple(repo, cache):
        oldhash, oldobsstore = cache['obsstore']
        ....

Choice A looks simple. Choice B is more expressive while the function body
could be out of control - ex. it could skip setting cache['obsstore'] even
if it says so in its decorator.

While Choice A may serve most requirements just well, I think "yield" is
more expressive, and avoid unnecessary indentation. For example, the
following could not be expressed using "return" cleanly and easily:

    - Optional dependency:

          if fastpath_cannot_work:
             yield preload.require('changelog')

      Maybe "raise MissingRequirement('changelog')", but that makes
      the function "restarted", while the generator could continue
      executing without losing context.

    - Optionally change the cache key:

      As mentioned in "4)", [1]

Therefore I think the generator way avoids all kinds of indentations
(reminds me of nodejs callback hell), and has better extensibility. So I
currently prefer it. Otherwise the "Choice A" may be good enough for now.
The code base has already used "yield" for its future implementation used
for batched remote peer calls.

[1] https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-February/092584.html

>         return hash, loadindex
> 
> BTW, when will 'f' be closed if we take the generator-based abstraction?
>
>     def preloadindex(repo):
>         f = repo.svfs('changelog.i')
>         try:
>             yield ...
>             yield ...
>         finally:
>             f.close()