| Submitter | Pierre-Yves David |
|---|---|
| Date | Dec. 19, 2012, 1:53 p.m. |
| Message ID | <56c016dea0622854146f.1355925197@crater1.logilab.fr> |
| Download | mbox | patch |
| Permalink | /patch/192/ |
| State | Superseded, archived |
| Headers | show |
Comments
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
On Wed, Dec 19, 2012 at 10:24:24PM -0600, Kevin Bullock wrote: > 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.] we usually revert to "lasted" revision as tip or tip-most. I think _cacheabletip is fine here. I fixed other feedback thanks. > > - 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. Hum lrev should have been update to the value of catip after the first update. I fixed that and added a comment in the newer version to make it more explicite.
Patch
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) - self._writebranchcache(partial, self.changelog.tip(), tiprev) - return partial @unfilteredmethod # Until we get a smarter cache management def updatebranchcache(self): tip = self.changelog.tip()