[PATCH 3 of 3] revset: fast implementation for fullreposet.__and__

Pierre-Yves David pierre-yves.david at ens-lyon.org
Mon Sep 22 19:50:11 CDT 2014



On 09/22/2014 05:10 PM, Durham Goode wrote:
>
> On 9/19/14, 5:07 PM, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david at fb.com>
>> # Date 1398878911 25200
>> #      Wed Apr 30 10:28:31 2014 -0700
>> # Node ID f9806f19d2d158c228ae7207371afa42475ed946
>> # Parent  5aec2be794fbd94adc4f624c57cdd2fc9907e6e3
>> revset: fast implementation for fullreposet.__and__
>>
>> "And" operation with something that contains the whole repo should be
>> super
>> cheap. Check method docstring for details.
>>
>> This provide massive boost to simple revset that use `subset & xxx`
>>
>> revset #0: p1(20000)
>> 0) wall 0.002447 comb 0.010000 user 0.010000 sys 0.000000 (best of 767)
>> 1) wall 0.000529 comb 0.000000 user 0.000000 sys 0.000000 (best of 3947)
>>
>> revset #1: p2(10000)
>> 0) wall 0.002464 comb 0.000000 user 0.000000 sys 0.000000 (best of 913)
>> 1) wall 0.000530 comb 0.000000 user 0.000000 sys 0.000000 (best of 4226)
>>
>> No other regression spotted.
>>
>> More performance are expected  in the future as more revset predicate are
>> converted to use `subset & xxx`
>>
>> diff --git a/mercurial/revset.py b/mercurial/revset.py
>> --- a/mercurial/revset.py
>> +++ b/mercurial/revset.py
>> @@ -2865,7 +2865,29 @@ class fullreposet(_spanset):
>>       """
>>       def __init__(self, repo):
>>           super(fullreposet, self).__init__(repo)
>> +    def __and__(self, other):
>> +        """fullrepo & other -> other
>> +
>> +        As self contains the whole repo, all of the other set should
>> also be in
>> +        self. Therefor `self & other = other`.
>> +
>> +        This boldly assume the other contains valid revs only.
>>
> I feel like other revsets may depend on the spanset to do the hidden
> commit filtering, and this would remove that.  Have you run the
> evolution tests with this enabled?

So the obsolete() (and related revset are subject to this error) It 
won't be too hard to fix for then.

However we can also think of a cleaner way to handle this. Smartset 
class. with content known to be pure could have a "smartset.pure == 
True" that trigger the optimisation.

I'm not so concerned about having this issue in the revset code itself. 
I'm more concerned about having people using the return of `repo.revs()` 
and receiving a fullreposet. So another option would be to disable this 
optimization before returning it to the "user"

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list