[PATCH STABLE] rebase: improve base revset performance

Durham Goode durham at fb.com
Tue Oct 21 11:49:50 CDT 2014


On 10/20/14 10:25 PM, Pierre-Yves David wrote:
>
>
> On 10/20/2014 07:11 PM, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham at fb.com>
>> # Date 1413856209 25200
>> #      Mon Oct 20 18:50:09 2014 -0700
>> # Node ID 1a9990727f5d75d70bb0d00fdffac571449574b6
>> # Parent  67cb1ab1ad1db6a66b96ac0c0333256b5383a3a2
>> rebase: improve base revset performance
>>
>> The old revset had pretty terrible performance on large repositories 
>> (12+
>> seconds). This new revset achieves the same result in only 0.7s. As 
>> we improve
>> the underlying revset APIs we can probably get this revset down to 
>> 'only(base,
>> dest)::', but at the moment that version still takes 2s.
>>
>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>> --- a/hgext/rebase.py
>> +++ b/hgext/rebase.py
>> @@ -272,9 +272,9 @@ def rebase(ui, repo, **opts):
>>                       ui.status(_('empty "base" revision set - '
>>                                   "can't compute rebase set\n"))
>>                       return 1
>> -                rebaseset = repo.revs(
>> -                    '(children(ancestor(%ld, %d)) and ::(%ld))::',
>> -                    base, dest, base)
>> +                commonanc = repo.revs('ancestor(%ld, %d)', base, 
>> dest).first()
>> +                rebaseset = repo.revs('(%d::(%ld) - %d)::',
>> +                                      commonanc, base, commonanc)
>
> So we suffer here from the cost of `::(X)` and the unability of 
> storing the ancestors call in a variable.
>
The common ancestor is very cheap to compute, so storing it in a 
variable was just so I didn't have to duplicate the ancestor() bit in 
the revset in an ugly way, not really for performance.
> I wonder if if we can get similar performance by limiting the walk of 
> `::(X)` with the min of the subset.
>
In theory the revset code should be smart enough to do this efficiently, 
but that change is too risky for the freeze.
> There is probably no value for going through revset got the ancestors 
> call. We could call the ancestors code directly.
>
I wanted to minimize the deviation from the old code.  Using 
ancestor.whatever() might have subtle differences I'm not aware of.
> Not sure about stable suitability.
I'll let Matt decide.  He seemed interested in taking my old original 
fix for stable before the freeze, so I figured this would qualify.


More information about the Mercurial-devel mailing list