Patchwork [V2] graphmod: compute slow revset query once prior to reachableroots (issue4782)

login
register
mail settings
Submitter Yuya Nishihara
Date Sept. 8, 2015, 3:22 p.m.
Message ID <5c20f2034fffbafd406c.1441725738@mimosa>
Download mbox | patch
Permalink /patch/10416/
State Accepted
Delegated to: Augie Fackler
Headers show

Comments

Yuya Nishihara - Sept. 8, 2015, 3:22 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1441720844 -32400
#      Tue Sep 08 23:00:44 2015 +0900
# Node ID 5c20f2034fffbafd406c0e4b1e7bbef9b3973965
# Parent  2f77cf0385ace2539f313da8132e05d03841d846
graphmod: compute slow revset query once prior to reachableroots (issue4782)

Because revsets query is evaluated lazily, "list(revs)" may take long for
complicated query. So we shouldn't iterate revs many times. This patch is the
easiest workaround for the issue4782. We could introduce more aggressive
caching, but it wouldn't be as fast as the simple baseset operation.

Gregory Szorc said "this makes `hg wip` on my Firefox clone ~4x faster than
3.5.1 (~6.5s to ~1.5s). This is after a regression in @ to ~45s."
Augie Fackler - Sept. 8, 2015, 5:03 p.m.
On Wed, Sep 09, 2015 at 12:22:18AM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1441720844 -32400
> #      Tue Sep 08 23:00:44 2015 +0900
> # Node ID 5c20f2034fffbafd406c0e4b1e7bbef9b3973965
> # Parent  2f77cf0385ace2539f313da8132e05d03841d846
> graphmod: compute slow revset query once prior to reachableroots (issue4782)

queued, but should this be on stable rather than default?

(it's on default for now)

>
> Because revsets query is evaluated lazily, "list(revs)" may take long for
> complicated query. So we shouldn't iterate revs many times. This patch is the
> easiest workaround for the issue4782. We could introduce more aggressive
> caching, but it wouldn't be as fast as the simple baseset operation.
>
> Gregory Szorc said "this makes `hg wip` on my Firefox clone ~4x faster than
> 3.5.1 (~6.5s to ~1.5s). This is after a regression in @ to ~45s."
>
> diff --git a/mercurial/graphmod.py b/mercurial/graphmod.py
> --- a/mercurial/graphmod.py
> +++ b/mercurial/graphmod.py
> @@ -260,6 +260,10 @@ def dagwalker(repo, revs):
>          for mpar in mpars:
>              gp = gpcache.get(mpar)
>              if gp is None:
> +                # precompute slow query as we know reachableroots() goes
> +                # through all revs (issue4782)
> +                if not isinstance(revs, revset.baseset):
> +                    revs = revset.baseset(revs)
>                  gp = gpcache[mpar] = revset.reachableroots(repo, revs, [mpar])
>              if not gp:
>                  parents.append(mpar)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Sept. 8, 2015, 11:01 p.m.
On 09/08/2015 10:03 AM, Augie Fackler wrote:
> On Wed, Sep 09, 2015 at 12:22:18AM +0900, Yuya Nishihara wrote:
>> # HG changeset patch
>> # User Yuya Nishihara <yuya@tcha.org>
>> # Date 1441720844 -32400
>> #      Tue Sep 08 23:00:44 2015 +0900
>> # Node ID 5c20f2034fffbafd406c0e4b1e7bbef9b3973965
>> # Parent  2f77cf0385ace2539f313da8132e05d03841d846
>> graphmod: compute slow revset query once prior to reachableroots (issue4782)
>
> queued, but should this be on stable rather than default?

The regression comes from Laurent's introduction of reachable roots (and 
many others contribution to get it to work). So stable should be unaffected.

Patch

diff --git a/mercurial/graphmod.py b/mercurial/graphmod.py
--- a/mercurial/graphmod.py
+++ b/mercurial/graphmod.py
@@ -260,6 +260,10 @@  def dagwalker(repo, revs):
         for mpar in mpars:
             gp = gpcache.get(mpar)
             if gp is None:
+                # precompute slow query as we know reachableroots() goes
+                # through all revs (issue4782)
+                if not isinstance(revs, revset.baseset):
+                    revs = revset.baseset(revs)
                 gp = gpcache[mpar] = revset.reachableroots(repo, revs, [mpar])
             if not gp:
                 parents.append(mpar)