[PATCH 2 of 2] revset: make optimizer use the new minusset

Durham Goode durham at fb.com
Thu Feb 11 13:44:20 EST 2016



On 2/11/16 10:14 AM, Martin von Zweigbergk wrote:
> I'm new to the revset code, so please forgive me if the below makes no sense.
>
> On Wed, Feb 10, 2016 at 4:23 PM, Durham Goode <durham at fb.com> wrote:
>> # HG changeset patch
>> # User Durham Goode <durham at fb.com>
>> # Date 1455146394 28800
>> #      Wed Feb 10 15:19:54 2016 -0800
>> # Node ID ec633f772ea500d3db5e2fff3240099960f679b3
>> # Parent  edfc4fda0c1ca78186d9a779f0d4d0cfc6e1c03f
>> revset: make optimizer use the new minusset
>>
>> Now that we have a minusset class, we need to change the optimizer to use it.
> It seems to me that these two patches do two things:
>
> 1) Tell the optimizer to use use the revset's '-' operator (i.e.
> abstractsmartset.__sub__()) when the user used '-' in the expression.
> These are the changes to revset.py in this patch, except for the the
> hunk starting on line 2846.
>
> 2) Return your new minusset class when using revset's '-' operator.
>
>  From what I can tell, step 1) makes a big difference on its own. On
> the set of tests I ran, I did not see any benefit from step 2), but it
> might be that I didn't run the relevant tests. Would it make sense to
> restructure these patches into those two steps instead? It would be
> nice to see timings showing improvements from both steps.
So the difference is that the minusset class has a bit more caching 
available.  For instance, len(minusset) computes a list which is then 
used to answer future questions, whereas filteredset (the current '-' 
operator implementaiton) recomputes the length each time.  I originally 
approached the problem from the addset angle (copy that and remove 
code), but I guess I could add better caching to the filteredset and 
achieve the same result in this case.

Let me run the numbers without minusset.  If the lack of caching doesn't 
end up mattering much, I'll just resend without that part and we can add 
caching to filteredset later.
> Also, please rename either the class or the function, so they're not
> both called "minusset". I don't even know which one Python will pick
> when they share a name.
Will do.  The only reason this even worked on accident was because the 
minusset function gets put into the name->func dictionary before the 
class is set, then never used again.


More information about the Mercurial-devel mailing list