Patchwork [5,of,5] phases: retractboundary should update all affected phases

login
register
mail settings
Submitter Durham Goode
Date Oct. 9, 2014, 7:22 p.m.
Message ID <be5f4f8f38440da11821.1412882563@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/6169/
State Changes Requested
Headers show

Comments

Durham Goode - Oct. 9, 2014, 7:22 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1412707733 25200
#      Tue Oct 07 11:48:53 2014 -0700
# Node ID be5f4f8f38440da11821946ae84abfb199190656
# Parent  80710688335b3e647bbf74700cb6e1a464b909b2
phases: retractboundary should update all affected phases

Previously retractboundary only updated the roots for the phase it was changing.
This meant there might be other roots in other phases that were now incorrect.
It just happened to work fine, because the old algorithms allowed future phases
to overwrite the results of earlier phases.  The upcoming new phase algorithm
requires the roots to be correct, so this patch fixes up all roots affected by
the boundary retracting.
Durham Goode - Oct. 13, 2014, 7:34 p.m.
On 10/12/14 11:33 PM, Pierre-Yves David wrote:
>
>
> On 10/09/2014 12:22 PM, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1412707733 25200
>> #      Tue Oct 07 11:48:53 2014 -0700
>> # Node ID be5f4f8f38440da11821946ae84abfb199190656
>> # Parent  80710688335b3e647bbf74700cb6e1a464b909b2
>> phases: retractboundary should update all affected phases
>
>>
>> diff --git a/mercurial/phases.py b/mercurial/phases.py
>> --- a/mercurial/phases.py
>> +++ b/mercurial/phases.py
>> @@ -287,7 +287,18 @@ class phasecache(object):
>>                   raise util.Abort(_('cannot change null revision 
>> phase'))
>>               currentroots = currentroots.copy()
>>               currentroots.update(newroots)
>> -            ctxs = repo.set('roots(%ln::)', currentroots)
>> +            affectedrevs = repo.revs('%ln::', currentroots)
>> +            for phase, roots in enumerate(self.phaseroots):
>> +                if targetphase <= phase:
>> +                    continue
>> +                roots = roots.copy()
>> +                for rev in affectedrevs:
>> +                    node = repo.changelog.node(rev)
>
> As we are at it. testing the other way around (iterating over roots) 
> may be faster for huge phases movement.
>
I thought about this, but I'd rather have the operation be O(size of 
affected revs) instead of O(size of local commits in repo).
>> +                    if node in roots:
>> +                        roots.discard(node)
>> +                self._updateroots(phase, roots, tr)
Durham Goode - Oct. 14, 2014, 1:55 a.m.
On 10/9/14 12:22 PM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1412707733 25200
> #      Tue Oct 07 11:48:53 2014 -0700
> # Node ID be5f4f8f38440da11821946ae84abfb199190656
> # Parent  80710688335b3e647bbf74700cb6e1a464b909b2
> phases: retractboundary should update all affected phases
>
> Previously retractboundary only updated the roots for the phase it was changing.
> This meant there might be other roots in other phases that were now incorrect.
> It just happened to work fine, because the old algorithms allowed future phases
> to overwrite the results of earlier phases.  The upcoming new phase algorithm
> requires the roots to be correct, so this patch fixes up all roots affected by
> the boundary retracting.
Pierre-Yves and I think this series (and the subsequent two that will 
use it) are probably too risky to get in right before the freeze.  We'll 
try to deploy them internally to get an idea of performance cost, then 
get them upstream after the freeze.

Patch

diff --git a/mercurial/phases.py b/mercurial/phases.py
--- a/mercurial/phases.py
+++ b/mercurial/phases.py
@@ -287,7 +287,18 @@  class phasecache(object):
                 raise util.Abort(_('cannot change null revision phase'))
             currentroots = currentroots.copy()
             currentroots.update(newroots)
-            ctxs = repo.set('roots(%ln::)', currentroots)
+            affectedrevs = repo.revs('%ln::', currentroots)
+            for phase, roots in enumerate(self.phaseroots):
+                if targetphase <= phase:
+                    continue
+                roots = roots.copy()
+                for rev in affectedrevs:
+                    node = repo.changelog.node(rev)
+                    if node in roots:
+                        roots.discard(node)
+                self._updateroots(phase, roots, tr)
+
+            ctxs = repo.set('roots(%ld)', affectedrevs)
             currentroots.intersection_update(ctx.node() for ctx in ctxs)
             self._updateroots(targetphase, currentroots, tr)
         repo.invalidatevolatilesets()