Patchwork [2,of,2] revset: fix merge() to fall back to changectx API if wdir specified

login
register
mail settings
Submitter Yuya Nishihara
Date June 9, 2019, 1:54 p.m.
Message ID <9b8e1a2d5b0c192dd118.1560088477@mimosa>
Download mbox | patch
Permalink /patch/40389/
State Accepted
Headers show

Comments

Yuya Nishihara - June 9, 2019, 1:54 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1560086621 -32400
#      Sun Jun 09 22:23:41 2019 +0900
# Node ID 9b8e1a2d5b0c192dd11803a8413d2e4795eb2559
# Parent  282376c97e4ff13a310c8688eb1e9f393f41b3a3
revset: fix merge() to fall back to changectx API if wdir specified

I have a code which basically runs "0:wdir() & <user-revset>", and it crashed
if merge() were passed in.
Pierre-Yves David - June 10, 2019, 10:04 p.m.
The patch looks functionnally good. Do we have any information on the 
performance impact ?

On 6/9/19 3:54 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1560086621 -32400
> #      Sun Jun 09 22:23:41 2019 +0900
> # Node ID 9b8e1a2d5b0c192dd11803a8413d2e4795eb2559
> # Parent  282376c97e4ff13a310c8688eb1e9f393f41b3a3
> revset: fix merge() to fall back to changectx API if wdir specified
> 
> I have a code which basically runs "0:wdir() & <user-revset>", and it crashed
> if merge() were passed in.
> 
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1362,8 +1362,12 @@ def merge(repo, subset, x):
>       getargs(x, 0, 0, _("merge takes no arguments"))
>       cl = repo.changelog
>       nullrev = node.nullrev
> -    return subset.filter(lambda r: cl.parentrevs(r)[1] != nullrev,
> -                         condrepr='<merge>')
> +    def ismerge(r):
> +        try:
> +            return cl.parentrevs(r)[1] != nullrev
> +        except error.WdirUnsupported:
> +            return bool(repo[r].p2())
> +    return subset.filter(ismerge, condrepr='<merge>')
>   
>   @predicate('branchpoint()', safe=True)
>   def branchpoint(repo, subset, x):
> diff --git a/tests/test-revset.t b/tests/test-revset.t
> --- a/tests/test-revset.t
> +++ b/tests/test-revset.t
> @@ -2076,6 +2076,17 @@ itself isn't returned unless it is expli
>     $ log 'parents(merge())'
>     4
>     5
> +
> +  $ hg merge 7
> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  (branch merge, don't forget to commit)
> +  $ log '0:wdir() & merge()'
> +  6
> +  2147483647
> +  $ hg update -qC .
> +  $ log '0:wdir() & merge()'
> +  6
> +
>     $ log 'p1(branchpoint())'
>     0
>     2
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - June 10, 2019, 11:32 p.m.
On Tue, 11 Jun 2019 00:04:19 +0200, Pierre-Yves David wrote:
> The patch looks functionnally good. Do we have any information on the
> performance impact ?

Appears no. IIRC, there are one (or two?) extra ops per loop to open
try-except context.

% echo 'merge()' | HGPLAIN=1 ./contrib/revsetbenchmarks.py --variants=plain -R ../mercurial '@:9b8e1a2d5b0c'
Revision:
0) Revision  69900:f163e2b2594c: phabricator: pass ui instead of repo to callconduit
1) Revision  69901:282376c97e4f: revset: use nullrev constant in merge()
2) Revision  (tip) 69903:9b8e1a2d5b0c: revset: fix merge() to fall back to changectx API if wdir specified


revset #0: merge()
   plain
0) 0.050687
1) 0.050049
2) 0.051622
Pierre-Yves David - June 11, 2019, 1:23 p.m.
Okay, that looks good to me then.

On 6/11/19 1:32 AM, Yuya Nishihara wrote:
> On Tue, 11 Jun 2019 00:04:19 +0200, Pierre-Yves David wrote:
>> The patch looks functionnally good. Do we have any information on the
>> performance impact ?
> 
> Appears no. IIRC, there are one (or two?) extra ops per loop to open
> try-except context.
> 
> % echo 'merge()' | HGPLAIN=1 ./contrib/revsetbenchmarks.py --variants=plain -R ../mercurial '@:9b8e1a2d5b0c'
> Revision:
> 0) Revision  69900:f163e2b2594c: phabricator: pass ui instead of repo to callconduit
> 1) Revision  69901:282376c97e4f: revset: use nullrev constant in merge()
> 2) Revision  (tip) 69903:9b8e1a2d5b0c: revset: fix merge() to fall back to changectx API if wdir specified
> 
> 
> revset #0: merge()
>     plain
> 0) 0.050687
> 1) 0.050049
> 2) 0.051622
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1362,8 +1362,12 @@  def merge(repo, subset, x):
     getargs(x, 0, 0, _("merge takes no arguments"))
     cl = repo.changelog
     nullrev = node.nullrev
-    return subset.filter(lambda r: cl.parentrevs(r)[1] != nullrev,
-                         condrepr='<merge>')
+    def ismerge(r):
+        try:
+            return cl.parentrevs(r)[1] != nullrev
+        except error.WdirUnsupported:
+            return bool(repo[r].p2())
+    return subset.filter(ismerge, condrepr='<merge>')
 
 @predicate('branchpoint()', safe=True)
 def branchpoint(repo, subset, x):
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -2076,6 +2076,17 @@  itself isn't returned unless it is expli
   $ log 'parents(merge())'
   4
   5
+
+  $ hg merge 7
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  (branch merge, don't forget to commit)
+  $ log '0:wdir() & merge()'
+  6
+  2147483647
+  $ hg update -qC .
+  $ log '0:wdir() & merge()'
+  6
+
   $ log 'p1(branchpoint())'
   0
   2