[PATCH 01 of 10] branchmap: simplify _branchtags using a new _cachabletip method

[Kevin Bullock]

On Dec 19, 2012, at 7:53 AM, pierre-yves.david at logilab.fr wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1355923841 -3600
> # Node ID 56c016dea0622854146f52acb9b3d5c9258636b9
> # Parent  9c76da468a19a0cf9551b8a667d2913b0b0ee7be
> branchmap: simplify _branchtags using a new _cachabletip method

[Compulsive copy-editing follows. Sorry, I can't help it.]
Should be _cacheabletip (note the 'e'). [Google returns 2.5e6 results with the 'e', only 430k without it. Looks weird to me without it. Oh, also, we use 'cacheable' in scmutil.]

Actually, now reading what the method does, maybe something more like '_lastcacheable' ?

> The current _branchtags method is a remain of an older much larger function. Its
                                      remnant      older, much
[Bonus points for using the correct 'its' though. Most native speakers get that consistently wrong.]

> only remaining role is to help MQ to alter the version of branchcache we store
> on disk. As MQ mutate the repository it ensures persistent cache does not
                 mutates
> contains anything it is likely to alter.
  contain

> This changeset explicits the stable vs volatile part of repository and reduces the
                 makes explicit[?]
> MQ specific code to the computation of this limit. The main _branchtags code now
> handles this possible limits in all case.
                        limit         cases.

> This will helps to extract the branchmap logic from the repository.
            help to

> The new code of _branchtags is a bit duplicated, but as I expect major
> refactoring of this section I'm not keen to setup factorisation function here.
> 
> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -3475,11 +3475,11 @@ def reposetup(ui, repo):
>                 else:
>                     tags[patch[1]] = patch[0]
> 
>             return result
> 
> -        def _branchtags(self, partial, lrev):
> +        def _cachabletip(self):
>             q = self.mq
>             cl = self.changelog
>             qbase = None
>             if not q.applied:
>                 if getattr(self, '_committingpatch', False):
> @@ -3490,29 +3490,14 @@ def reposetup(ui, repo):
>                 try:
>                     qbase = self.unfiltered().changelog.rev(qbasenode)
>                 except error.LookupError:
>                     self.ui.warn(_('mq status file refers to unknown node %s\n')
>                                  % short(qbasenode))
> -            if qbase is None:
> -                return super(mqrepo, self)._branchtags(partial, lrev)
> -
> -            start = lrev + 1
> -            if start < qbase:
> -                # update the cache (excluding the patches) and save it
> -                ctxgen = (self[r] for r in xrange(lrev + 1, qbase))
> -                self._updatebranchcache(partial, ctxgen)
> -                self._writebranchcache(partial, cl.node(qbase - 1), qbase - 1)
> -                start = qbase
> -            # if start = qbase, the cache is as updated as it should be.
> -            # if start > qbase, the cache includes (part of) the patches.
> -            # we might as well use it, but we won't save it.
> -
> -            # update the cache up to the tip
> -            ctxgen = (self[r] for r in xrange(start, len(cl)))
> -            self._updatebranchcache(partial, ctxgen)
> -
> -            return partial
> +            ret = super(mqrepo, self)._cachabletip()
> +            if qbase is not None:
> +                ret = min(qbase - 1, ret)
> +            return ret
> 
>     if repo.local():
>         repo.__class__ = mqrepo
> 
>         repo._phasedefaults.append(mqphasedefaults)
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -630,18 +630,38 @@ class localrepository(object):
>         for bookmark, n in self._bookmarks.iteritems():
>             if n == node:
>                 marks.append(bookmark)
>         return sorted(marks)
> 
> +    def _cachabletip(self):
> +        """tip-most revision stable enought to used in persistent cache
> +
> +        This function is overwritten by MQ to ensure we do not write cache for
> +        a part of the history that will likely change.
> +
> +        Efficient handling of filtered revision in branchcache should offer a
> +        better alternative. But we are using this approach until it is ready.
> +        """
> +        cl = self.changelog
> +        return cl.rev(cl.tip())
> +
>     def _branchtags(self, partial, lrev):
>         # TODO: rename this function?
> +        cl = self.changelog
> +        catip = self._cachabletip()
> +        # if lrev == catip: cache is already up to date
> +        # if lrev >  catip: we have uncachable element in `partial` can't write
> +        #                   on disk
> +        if lrev < catip:
> +            ctxgen = (self[r] for r in cl.revs(lrev + 1, catip))
> +            self._updatebranchcache(partial, ctxgen)
> +            self._writebranchcache(partial, cl.node(catip), catip)
> +        # update cache up to current tip
>         tiprev = len(self) - 1
> -        if lrev != tiprev:
> -            ctxgen = (self[r] for r in self.changelog.revs(lrev + 1, tiprev))
> +        if lrev < tiprev:
> +            ctxgen = (self[r] for r in cl.revs(lrev + 1, tiprev))
>             self._updatebranchcache(partial, ctxgen)
Why are we doing this again? I'm not suggesting it be factored out, just wondering why we're updating the branchcache for tip, _without_ writing it to disk here.

> -            self._writebranchcache(partial, self.changelog.tip(), tiprev)
> -
>         return partial
> 
>     @unfilteredmethod # Until we get a smarter cache management
>     def updatebranchcache(self):
>         tip = self.changelog.tip()
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock