Patchwork [3,of,5] transplant: use set for prune lookup

login
register
mail settings
Submitter Mads Kiilerich
Date April 16, 2013, 5:49 p.m.
Message ID <3f621c427c998d049888.1366134549@mk-desktop>
Download mbox | patch
Permalink /patch/1354/
State Accepted
Commit 0fc41f88f1482e24f4d74bc25670c03a6fc555fb
Headers show

Comments

Mads Kiilerich - April 16, 2013, 5:49 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1366133519 -7200
#      Tue Apr 16 19:31:59 2013 +0200
# Node ID 3f621c427c998d049888ab60e6f40b7258f9f65f
# Parent  7b1520771113094fdbe19511ef7ef22f2d2666f2
transplant: use set for prune lookup
Augie Fackler - April 17, 2013, 1:59 p.m.
On Tue, Apr 16, 2013 at 07:49:09PM +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1366133519 -7200
> #      Tue Apr 16 19:31:59 2013 +0200
> # Node ID 3f621c427c998d049888ab60e6f40b7258f9f65f
> # Parent  7b1520771113094fdbe19511ef7ef22f2d2666f2
> transplant: use set for prune lookup

Before/after perf numbers? For small sets sometimes the list can be
better than the set? Does fb care about this, and maybe they have a
performance dashboard for it?

>
> diff --git a/hgext/transplant.py b/hgext/transplant.py
> --- a/hgext/transplant.py
> +++ b/hgext/transplant.py
> @@ -633,8 +633,8 @@
>
>          tf = tp.transplantfilter(repo, source, p1)
>          if opts.get('prune'):
> -            prune = [source.lookup(r)
> -                     for r in scmutil.revrange(source, opts.get('prune'))]
> +            prune = set(source.lookup(r)
> +                        for r in scmutil.revrange(source, opts.get('prune')))
>              matchfn = lambda x: tf(x) and x not in prune
>          else:
>              matchfn = tf
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Mads Kiilerich - April 17, 2013, 2:12 p.m.
On 04/17/2013 03:59 PM, Augie Fackler wrote:
> On Tue, Apr 16, 2013 at 07:49:09PM +0200, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1366133519 -7200
>> #      Tue Apr 16 19:31:59 2013 +0200
>> # Node ID 3f621c427c998d049888ab60e6f40b7258f9f65f
>> # Parent  7b1520771113094fdbe19511ef7ef22f2d2666f2
>> transplant: use set for prune lookup
> Before/after perf numbers? For small sets sometimes the list can be
> better than the set? Does fb care about this, and maybe they have a
> performance dashboard for it?

I haven't done any measurements. I stumbled upon it when looking through 
the code. It seemed like something that obviously used the wrong pattern.

The revrange will only be converted to a set once - that overhead should 
be negligible no matter what. Using a set will change a rebase of n 
changesets while pruning p changesets from O(p+n*p) to O(p+p+n).

/Mads


>
>> diff --git a/hgext/transplant.py b/hgext/transplant.py
>> --- a/hgext/transplant.py
>> +++ b/hgext/transplant.py
>> @@ -633,8 +633,8 @@
>>
>>           tf = tp.transplantfilter(repo, source, p1)
>>           if opts.get('prune'):
>> -            prune = [source.lookup(r)
>> -                     for r in scmutil.revrange(source, opts.get('prune'))]
>> +            prune = set(source.lookup(r)
>> +                        for r in scmutil.revrange(source, opts.get('prune')))
>>               matchfn = lambda x: tf(x) and x not in prune
>>           else:
>>               matchfn = tf
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - April 17, 2013, 8:43 p.m.
On Wed, 2013-04-17 at 09:59 -0400, Augie Fackler wrote:
> On Tue, Apr 16, 2013 at 07:49:09PM +0200, Mads Kiilerich wrote:
> > # HG changeset patch
> > # User Mads Kiilerich <madski@unity3d.com>
> > # Date 1366133519 -7200
> > #      Tue Apr 16 19:31:59 2013 +0200
> > # Node ID 3f621c427c998d049888ab60e6f40b7258f9f65f
> > # Parent  7b1520771113094fdbe19511ef7ef22f2d2666f2
> > transplant: use set for prune lookup
> 
> Before/after perf numbers? For small sets sometimes the list can be
> better than the set? Does fb care about this, and maybe they have a
> performance dashboard for it?

Nope. FB's favorite hammer is rebase.
Matt Mackall - April 18, 2013, 5:39 a.m.
On Tue, 2013-04-16 at 19:49 +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1366133519 -7200
> #      Tue Apr 16 19:31:59 2013 +0200
> # Node ID 3f621c427c998d049888ab60e6f40b7258f9f65f
> # Parent  7b1520771113094fdbe19511ef7ef22f2d2666f2
> transplant: use set for prune lookup

Queued for default, thanks.

Patch

diff --git a/hgext/transplant.py b/hgext/transplant.py
--- a/hgext/transplant.py
+++ b/hgext/transplant.py
@@ -633,8 +633,8 @@ 
 
         tf = tp.transplantfilter(repo, source, p1)
         if opts.get('prune'):
-            prune = [source.lookup(r)
-                     for r in scmutil.revrange(source, opts.get('prune'))]
+            prune = set(source.lookup(r)
+                        for r in scmutil.revrange(source, opts.get('prune')))
             matchfn = lambda x: tf(x) and x not in prune
         else:
             matchfn = tf