Patchwork [3,of,5] rebase: replace revset with a manual computation

login
register
mail settings
Submitter Durham Goode
Date Nov. 12, 2013, 4:14 a.m.
Message ID <c22c16072cc8519f18a6.1384229665@dev350.prn1.facebook.com>
Download mbox | patch
Permalink /patch/2919/
State Superseded
Headers show

Comments

Durham Goode - Nov. 12, 2013, 4:14 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1384216660 28800
#      Mon Nov 11 16:37:40 2013 -0800
# Node ID c22c16072cc8519f18a66932c87f2892d3b266be
# Parent  176333a793ceaa0c2344d01b1d12b837ee43715c
rebase: replace revset with a manual computation

Revsets are slow on large repos.  Replacing this revset with the manual
computation makes amend go from 6 seconds to 5.5 seconds on a large repo.
David Soria Parra - Nov. 13, 2013, 6:20 a.m.
On 11/11/2013 08:14 PM, Durham Goode wrote:
> diff --git a/mercurial/repair.py b/mercurial/repair.py
> --- a/mercurial/repair.py
> +++ b/mercurial/repair.py
> @@ -7,7 +7,7 @@
>  # GNU General Public License version 2 or any later version.
>  
>  from mercurial import changegroup
> -from mercurial.node import short
> +from mercurial.node import short, nullrev
>  from mercurial.i18n import _
>  import os
>  import errno
> @@ -91,11 +91,17 @@
>      savebases = [cl.node(r) for r in saverevs]
>      stripbases = [cl.node(r) for r in tostrip]
>  
> -    # For a set s, max(parents(s) - s) is the same as max(heads(::s - s)), but
> -    # is much faster
> -    newbmtarget = repo.revs('max(parents(%ld) - (%ld))', tostrip, tostrip)
> -    if newbmtarget:
> -        newbmtarget = repo[newbmtarget[0]].node()
> +    # bookmarks should go to the max parent
> +    maxparent = nullrev
> +    cl = repo.changelog
> +    for rev in tostrip:
> +        for prev in cl.parentrevs(rev):
> +            if prev > maxparent and not prev in tostrip:
> +                maxparent = prev
> +
> +    newbmtarget = maxparent
> +    if newbmtarget != nullrev:
> +        newbmtarget = repo[newbmtarget].node()

Maybe a bit picky, but I think it might be better to have add a comment
saying this is an optimization of max(parents(tostrop) - (tostrop)), so
nobody wants to change it back to revset later.

David

Patch

diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -7,7 +7,7 @@ 
 # GNU General Public License version 2 or any later version.
 
 from mercurial import changegroup
-from mercurial.node import short
+from mercurial.node import short, nullrev
 from mercurial.i18n import _
 import os
 import errno
@@ -91,11 +91,17 @@ 
     savebases = [cl.node(r) for r in saverevs]
     stripbases = [cl.node(r) for r in tostrip]
 
-    # For a set s, max(parents(s) - s) is the same as max(heads(::s - s)), but
-    # is much faster
-    newbmtarget = repo.revs('max(parents(%ld) - (%ld))', tostrip, tostrip)
-    if newbmtarget:
-        newbmtarget = repo[newbmtarget[0]].node()
+    # bookmarks should go to the max parent
+    maxparent = nullrev
+    cl = repo.changelog
+    for rev in tostrip:
+        for prev in cl.parentrevs(rev):
+            if prev > maxparent and not prev in tostrip:
+                maxparent = prev
+
+    newbmtarget = maxparent
+    if newbmtarget != nullrev:
+        newbmtarget = repo[newbmtarget].node()
     else:
         newbmtarget = '.'