[PATCH 2 of 2] revset: make children only look at commits >= minrev

[Durham Goode]

On 9/23/14, 11:34 AM, Durham Goode wrote:
>
> On 9/23/14, 11:27 AM, Pierre-Yves David wrote:
>>
>>
>> On 09/23/2014 11:23 AM, Augie Fackler wrote:
>>> On Mon, Sep 22, 2014 at 11:31:09PM -0700, Durham Goode wrote:
>>>> # HG changeset patch
>>>> # User Durham Goode <durham at fb.com>
>>>> # Date 1411450748 25200
>>>> #      Mon Sep 22 22:39:08 2014 -0700
>>>> # Node ID 14087ac481b1d25ccaa69ed15a014f56c829da65
>>>> # Parent  e7e0c696c29e1735aad7a77f82bfc987354a98c1
>>>> revset: make children only look at commits >= minrev
>>>
>>> Nice! Both queued, thanks.
>>
>> Please don't. I really would like that the work on revset focus on 
>> fixing the core mechanism instead of working around its limitation by 
>> making the code more complex. Having more complex code will make it 
>> harder to get the benefit of improving the core mechanism.
> I don't think either of these make the code that much more complex, 
> and the performance win is pretty massive.  In particular, patch #2 
> uses the smartness of the subset to perform it's work more 
> efficiently, which is exactly what the classes are there to allow.
>
> As for patch #1, I still think we should be making all our revsets 
> 'localwork & subset' instead of 'subset & localwork' because it 
> logically indicates the better O(...).  For your fullreposet work, you 
> could just have fullreposet.__contains__ return True to provide good 
> performance (similar to your optimization for 'subset & localwork').
Pierre-Yves and I have come to a stalemate in real life, so perhaps the 
community will have an opinion.

I believe these changes are small enough and impactful enough to go in 
as is. They don't cause any regressions in the revset benchmarks, and 
are unlikely to cause significant regressions in any other cases.  While 
the same performance might be achievable by refactoring the underlying 
revset classes to be smarter, the current change is 3 lines, buys us a 
40% perf improvement, and doesn't make a future refactor any more difficult.