Patchwork [2,of,4] revset: remove useless code to reorder weights of 'or' expression

login
register
mail settings
Submitter Yuya Nishihara
Date May 26, 2015, 3 p.m.
Message ID <70591283d99dcc442f0e.1432652420@mimosa>
Download mbox | patch
Permalink /patch/9277/
State Changes Requested
Headers show

Comments

Yuya Nishihara - May 26, 2015, 3 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1430040452 -32400
#      Sun Apr 26 18:27:32 2015 +0900
# Node ID 70591283d99dcc442f0e7bdb9afa603eb18bfac1
# Parent  3c41bb6a51f96c0e4409f6b21689f61c16e149b2
revset: remove useless code to reorder weights of 'or' expression

Perhaps there was copy-paste error.
Matt Mackall - May 26, 2015, 4:14 p.m.
On Wed, 2015-05-27 at 00:00 +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1430040452 -32400
> #      Sun Apr 26 18:27:32 2015 +0900
> # Node ID 70591283d99dcc442f0e7bdb9afa603eb18bfac1
> # Parent  3c41bb6a51f96c0e4409f6b21689f61c16e149b2
> revset: remove useless code to reorder weights of 'or' expression
> 
> Perhaps there was copy-paste error.

There's an error here, but this is not the fix.

If we have an expression like

 "contains('set:grep(keyword)') or branch(default)"

..we can optimize this expression by finding everything on branch(foo)
first before grepping every file in every revision.

So if wb > wa, we should swap a and b.
Yuya Nishihara - May 26, 2015, 10:54 p.m.
On Tue, 26 May 2015 11:14:27 -0500, Matt Mackall wrote:
> On Wed, 2015-05-27 at 00:00 +0900, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1430040452 -32400
> > #      Sun Apr 26 18:27:32 2015 +0900
> > # Node ID 70591283d99dcc442f0e7bdb9afa603eb18bfac1
> > # Parent  3c41bb6a51f96c0e4409f6b21689f61c16e149b2
> > revset: remove useless code to reorder weights of 'or' expression
> > 
> > Perhaps there was copy-paste error.
> 
> There's an error here, but this is not the fix.
> 
> If we have an expression like
> 
>  "contains('set:grep(keyword)') or branch(default)"
> 
> ..we can optimize this expression by finding everything on branch(foo)
> first before grepping every file in every revision.
> 
> So if wb > wa, we should swap a and b.

Hmm, but doesn't it change the order of the result set?
I think "sort(A + B)" == "sort(B + A)", but "A + B" != "B + A".
Matt Mackall - May 26, 2015, 11:54 p.m.
On Wed, 2015-05-27 at 07:54 +0900, Yuya Nishihara wrote:
> On Tue, 26 May 2015 11:14:27 -0500, Matt Mackall wrote:
> > On Wed, 2015-05-27 at 00:00 +0900, Yuya Nishihara wrote:
> > > # HG changeset patch
> > > # User Yuya Nishihara <yuya@tcha.org>
> > > # Date 1430040452 -32400
> > > #      Sun Apr 26 18:27:32 2015 +0900
> > > # Node ID 70591283d99dcc442f0e7bdb9afa603eb18bfac1
> > > # Parent  3c41bb6a51f96c0e4409f6b21689f61c16e149b2
> > > revset: remove useless code to reorder weights of 'or' expression
> > > 
> > > Perhaps there was copy-paste error.
> > 
> > There's an error here, but this is not the fix.
> > 
> > If we have an expression like
> > 
> >  "contains('set:grep(keyword)') or branch(default)"
> > 
> > ..we can optimize this expression by finding everything on branch(foo)
> > first before grepping every file in every revision.
> > 
> > So if wb > wa, we should swap a and b.
> 
> Hmm, but doesn't it change the order of the result set?
> I think "sort(A + B)" == "sort(B + A)", but "A + B" != "B + A".

Good point. But at the same time, we have a & b doing the same sort of
swapping. For now, you should replace this with a note of the missed
opportunity.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2162,8 +2162,6 @@  def optimize(x, small):
     elif op == 'or':
         wa, ta = optimize(x[1], False)
         wb, tb = optimize(x[2], False)
-        if wb < wa:
-            wb, wa = wa, wb
         return max(wa, wb), (op, ta, tb)
     elif op == 'not':
         # Optimize not public() to _notpublic() because we have a fast version