Patchwork [8,of,8,demandimport-py3] python3: allow hgloader to work with lazy loaders

login
register
mail settings
Submitter Siddharth Agarwal
Date May 21, 2017, 8:48 p.m.
Message ID <902d8e08f7e5bc6114e2.1495399683@devvm31800.prn1.facebook.com>
Download mbox | patch
Permalink /patch/20802/
State Accepted
Headers show

Comments

Siddharth Agarwal - May 21, 2017, 8:48 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1495398377 25200
#      Sun May 21 13:26:17 2017 -0700
# Node ID 902d8e08f7e5bc6114e2fd9285be6ef75225da9e
# Parent  1bf07f6a0c323a4a842d7e76d14e4e893a2b68ca
python3: allow hgloader to work with lazy loaders

Don't clobber the loader returned from find_spec.

This brings `hg version` down from 0.27 seconds to 0.17.
Gregory Szorc - May 22, 2017, 1:26 a.m.
On Sun, May 21, 2017 at 1:48 PM, Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1495398377 25200
> #      Sun May 21 13:26:17 2017 -0700
> # Node ID 902d8e08f7e5bc6114e2fd9285be6ef75225da9e
> # Parent  1bf07f6a0c323a4a842d7e76d14e4e893a2b68ca
> python3: allow hgloader to work with lazy loaders
>
> Don't clobber the loader returned from find_spec.
>
> This brings `hg version` down from 0.27 seconds to 0.17.
>

Out of curiosity, is this with HGRCPATH=? On my machine, current `hg
version` on 3.6.0 on Linux on any i7-6700K is ~130ms. ~25ms with Python 2
(~60ms with demand import disabled). I suspect your machine is either much
slower or loading a bunch of extensions.


>
> diff --git a/mercurial/__init__.py b/mercurial/__init__.py
> --- a/mercurial/__init__.py
> +++ b/mercurial/__init__.py
> @@ -53,7 +53,14 @@ if sys.version_info[0] >= 3:
>
>              # TODO need to support loaders from alternate specs, like zip
>              # loaders.
> -            spec.loader = hgloader(spec.name, spec.origin)
> +            loader = hgloader(spec.name, spec.origin)
> +            # Can't use util.safehasattr here because that would require
> +            # importing util, and we're in import code.
> +            if hasattr(spec.loader, 'loader'): # hasattr-py3-only
> +                # This is a nested loader (maybe a lazy loader?)
> +                spec.loader.loader = loader
> +            else:
> +                spec.loader = loader
>              return spec
>
>      def replacetokens(tokens, fullname):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Siddharth Agarwal - May 22, 2017, 5:15 p.m.
On 5/21/17 18:26, Gregory Szorc wrote:
> On Sun, May 21, 2017 at 1:48 PM, Siddharth Agarwal <sid0@fb.com 
> <mailto:sid0@fb.com>> wrote:
>
>     # HG changeset patch
>     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
>     # Date 1495398377 25200
>     #      Sun May 21 13:26:17 2017 -0700
>     # Node ID 902d8e08f7e5bc6114e2fd9285be6ef75225da9e
>     # Parent  1bf07f6a0c323a4a842d7e76d14e4e893a2b68ca
>     python3: allow hgloader to work with lazy loaders
>
>     Don't clobber the loader returned from find_spec.
>
>     This brings `hg version` down from 0.27 seconds to 0.17.
>
>
> Out of curiosity, is this with HGRCPATH=? On my machine, current `hg 
> version` on 3.6.0 on Linux on any i7-6700K is ~130ms. ~25ms with 
> Python 2 (~60ms with demand import disabled). I suspect your machine 
> is either much slower or loading a bunch of extensions.

This was with HGRCPATH= and with 3.6.1. I'm not sure why the results are 
so different from yours, but it consistently takes 160-190ms.

- Siddharth

>
>     diff --git a/mercurial/__init__.py b/mercurial/__init__.py
>     --- a/mercurial/__init__.py
>     +++ b/mercurial/__init__.py
>     @@ -53,7 +53,14 @@ if sys.version_info[0] >= 3:
>
>                  # TODO need to support loaders from alternate specs,
>     like zip
>                  # loaders.
>     -            spec.loader = hgloader(spec.name <http://spec.name>,
>     spec.origin)
>     +            loader = hgloader(spec.name <http://spec.name>,
>     spec.origin)
>     +            # Can't use util.safehasattr here because that would
>     require
>     +            # importing util, and we're in import code.
>     +            if hasattr(spec.loader, 'loader'): # hasattr-py3-only
>     +                # This is a nested loader (maybe a lazy loader?)
>     +                spec.loader.loader = loader
>     +            else:
>     +                spec.loader = loader
>                  return spec
>
>          def replacetokens(tokens, fullname):
>     _______________________________________________
>     Mercurial-devel mailing list
>     Mercurial-devel@mercurial-scm.org
>     <mailto:Mercurial-devel@mercurial-scm.org>
>     https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>     <https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel>
>
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Jun Wu - May 27, 2017, 1:15 a.m.
Excerpts from Gregory Szorc's message of 2017-05-21 18:26:53 -0700:
> > This brings `hg version` down from 0.27 seconds to 0.17.
> 
> Out of curiosity, is this with HGRCPATH=? On my machine, current `hg
> version` on 3.6.0 on Linux on any i7-6700K is ~130ms. ~25ms with Python 2
> (~60ms with demand import disabled). I suspect your machine is either much
> slower or loading a bunch of extensions.

Maybe traceprof from fb-hgext could be helpful to find out why.

Patch

diff --git a/mercurial/__init__.py b/mercurial/__init__.py
--- a/mercurial/__init__.py
+++ b/mercurial/__init__.py
@@ -53,7 +53,14 @@  if sys.version_info[0] >= 3:
 
             # TODO need to support loaders from alternate specs, like zip
             # loaders.
-            spec.loader = hgloader(spec.name, spec.origin)
+            loader = hgloader(spec.name, spec.origin)
+            # Can't use util.safehasattr here because that would require
+            # importing util, and we're in import code.
+            if hasattr(spec.loader, 'loader'): # hasattr-py3-only
+                # This is a nested loader (maybe a lazy loader?)
+                spec.loader.loader = loader
+            else:
+                spec.loader = loader
             return spec
 
     def replacetokens(tokens, fullname):