Patchwork [3,of,3] revset: move lookup of first ancestor() candidate out of the loop

login
register
mail settings
Submitter Yuya Nishihara
Date June 28, 2018, 1:42 p.m.
Message ID <0005175b53bb3f829578.1530193376@mimosa>
Download mbox | patch
Permalink /patch/32482/
State Accepted
Headers show

Comments

Yuya Nishihara - June 28, 2018, 1:42 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1529159200 -32400
#      Sat Jun 16 23:26:40 2018 +0900
# Node ID 0005175b53bb3f829578757c1c34eab60c7c42a8
# Parent  fe4560fa5db2f1ee11cff233c55b7685e891517a
revset: move lookup of first ancestor() candidate out of the loop
via Mercurial-devel - June 28, 2018, 10:48 p.m.
On Thu, Jun 28, 2018 at 6:43 AM Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1529159200 -32400
> #      Sat Jun 16 23:26:40 2018 +0900
> # Node ID 0005175b53bb3f829578757c1c34eab60c7c42a8
> # Parent  fe4560fa5db2f1ee11cff233c55b7685e891517a
> revset: move lookup of first ancestor() candidate out of the loop
>

Queuing this series, thanks.

What's the motivation for this patch? Does it make some case faster or did
you just not like the condition inside the loop?
Yuya Nishihara - June 29, 2018, 11:32 a.m.
On Thu, 28 Jun 2018 15:48:59 -0700, Martin von Zweigbergk wrote:
> On Thu, Jun 28, 2018 at 6:43 AM Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1529159200 -32400
> > #      Sat Jun 16 23:26:40 2018 +0900
> > # Node ID 0005175b53bb3f829578757c1c34eab60c7c42a8
> > # Parent  fe4560fa5db2f1ee11cff233c55b7685e891517a
> > revset: move lookup of first ancestor() candidate out of the loop
> >
> 
> Queuing this series, thanks.
> 
> What's the motivation for this patch? Does it make some case faster or did
> you just not like the condition inside the loop?

Good question. I just feel initializing variable out of the loop is better.
There's no performance improvement.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -311,14 +311,15 @@  def ancestor(repo, subset, x):
     Will return empty list when passed no args.
     Greatest common ancestor of a single changeset is that changeset.
     """
-    anc = None
-    for r in orset(repo, fullreposet(repo), x, order=anyorder):
-        if anc is None:
-            anc = repo[r]
-        else:
-            anc = anc.ancestor(repo[r])
+    reviter = iter(orset(repo, fullreposet(repo), x, order=anyorder))
+    try:
+        anc = repo[next(reviter)]
+    except StopIteration:
+        return baseset()
+    for r in reviter:
+        anc = anc.ancestor(repo[r])
 
-    if anc is not None and anc.rev() in subset:
+    if anc.rev() in subset:
         return baseset([anc.rev()])
     return baseset()