[PATCH 1 of 3] localrepo: store closed state of each head in the branch cache

[Matt Mackall]

On Wed, 2012-02-08 at 19:17 +0800, Steven Brown wrote:
> On 8 February 2012 03:09, Matt Mackall <mpm at selenic.com> wrote:
> > On Tue, 2012-02-07 at 22:48 +0800, Steven Brown wrote:
> >> # HG changeset patch
> >> # User Steven Brown <StevenGBrown at gmail.com>
> >> # Date 1328623028 -28800
> >> # Node ID 0a5cacc501b4c680832c5ab9c775321611a6015c
> >> # Parent  8af9e08a094ff43f828085e1102b320995e0c1b2
> >> localrepo: store closed state of each head in the branch cache
> >>
> >> This will reduce the time taken to query the branches in a branchy repo.
> >> Profiling shows that although the closed state of a single revision can be
> >> determined very quickly, this is repeated many times and so it accounts for most
> >> of the time taken.
> >>
> >> The heads for each branch are still cached at .hg/cache/branchheads. The closed
> >> state of each head is now stored in the first column. When switching to this
> >> cset from an earlier version of Mercurial, or switching back again, the cache
> >> will be rebuilt.
> >
> > What happens on a repo shared on NFS by users with two different
> > versions? Our usual solution here is to move to a new cache name when we
> > tweak the format.
> 
> OK. 'heads' seems like a good name.
> 
> >> diff --git a/mercurial/discovery.py b/mercurial/discovery.py
> >> --- a/mercurial/discovery.py
> >> +++ b/mercurial/discovery.py
> >> @@ -183,12 +183,12 @@
> >>          # 4. Update newmap with outgoing changes.
> >>          # This will possibly add new heads and remove existing ones.
> >>          ctxgen = (repo[n] for n in outgoing.missing)
> >> -        repo._updatebranchcache(newmap, ctxgen)
> >> +        repo.updatebranchheads(newmap, ctxgen)
> >
> > Why are we changing the name of everything?
> 
> _updatebranchcache still updates the branch cache, but this now
> includes the closed state of the heads. The discovery module doesn't
> want this information, so it calls updatebranchheads to get the heads
> only.

Perhaps we want a completely separate cache/method just for closed
state.

> > Why is this no longer marked as an internal method?
> 
> It is called from discovery, so it isn't internal to localrepo. Have I
> misunderstood the naming convention?

Yes. a) the naming convention is advisory b) it suggests discovery has a
layering bug, which you're burying and c) the whole issue is orthogonal
to the thrust of your patch.

-- 
Mathematics is the supreme nostalgia of our time.