bpo-29585: optimize site.py startup time #136

methane · 2017-02-16T14:16:42Z

Skip importing sysconfig when possible.

Median +- std dev: [default] 15.8 ms +- 0.0 ms -> [patched] 14.7 ms +- 0.0 ms: 1.07x faster (-7%)

methane · 2017-02-16T14:32:10Z

$ cp  build/lib.linux-x86_64-3.7/_sysconfigdata_m_linux_x86_64-linux-gnu.py sysconfigdata
$ ./python -c 'import sysconfigdata'  # create pyc file
$ ./python -m timeit -s 'import sysconfigdata, importlib' -- 'importlib.reload(sysconfigdata)'
1000 loops, best of 5: 269 usec per loop

Since 'PYTHONFRAMEWORK' is in sysconfigdata, I cannot stop importing them.
But other cases, I can skip importing sysconfig and sysconfigdata completely.

ned-deily

This is a major change: by duplicating code from sysconfig.py into site.py, there would now be a implicit link between the two and a potential maintenance problem. If we were to do this, there should at least be mention of these duplications in sysconfig.py and/or tests for the duplicated behavior. Also, there should be a b.p.o issue for this proposed change.

vstinner

I dislike this approach. I suggest to experiment a _site extension module: see the issue.

methane · 2017-02-18T05:02:15Z

I've addes sys._framework and remove sysconf dependency completely.

Skip importing sysconfig when possible. Median +- std dev: [default] 15.8 ms +- 0.0 ms -> [patched] 14.7 ms +- 0.0 ms: 1.07x faster (-7%)

methane · 2017-06-28T10:44:18Z

@ned-deily I created bpo issue and added comment about duplicated code.

methane · 2017-06-28T10:48:29Z

@Haypo sysconfig (and _sysconfigdata_...) module is relatively large and
most applications (except packaging tools) doesn't use it at all.
Now this pull request is focusing it.

Slow code path is skipped by #167 although abspath is still slow.

vstinner · 2017-06-28T11:57:03Z

A quick & dirty benchmark using my perf module: I see an improvement of -0.8 ms (1.05x faster) on Python startup time.

haypo@selma$ ./python -m perf command --inherit=PYTHONPATH -v -o pr136.json -- ./python -c pass  
...
haypo@selma$ ./python -m perf command --inherit=PYTHONPATH -v -o ref.json -- ./python -c pass
...
haypo@selma$ ./python -m perf compare_to ref.json pr136.json 
Mean +- std dev: [ref] 17.4 ms +- 0.8 ms -> [pr136] 16.6 ms +- 1.1 ms: 1.05x faster (-5%)

EDIT: this benchmark was run on my Linux laptop.

vstinner · 2017-06-28T12:09:52Z

Hum, did I miss something? sysconfig is still imported by the site module on macOS by getsitepackages():

        if sys.platform == "darwin":
            # for framework builds *only* we add the standard Apple
            # locations.
            from sysconfig import get_config_var
            framework = get_config_var("PYTHONFRAMEWORK")
            if framework:
                sitepackages.append(
                        os.path.join("/Library", framework,
                            '%d.%d' % sys.version_info[:2], "site-packages"))

macbook:master haypo$ ./python.exe -c 'import sys; print("sysconfig" in sys.modules)'
True

I suggest this additionnal change:

diff --git a/Lib/site.py b/Lib/site.py
index 500c59b..929252d 100644
--- a/Lib/site.py
+++ b/Lib/site.py
@@ -334,15 +334,12 @@ def getsitepackages(prefixes=None):
         else:
             sitepackages.append(prefix)
             sitepackages.append(os.path.join(prefix, "lib", "site-packages"))
-        if sys.platform == "darwin":
-            # for framework builds *only* we add the standard Apple
-            # locations.
-            from sysconfig import get_config_var
-            framework = get_config_var("PYTHONFRAMEWORK")
-            if framework:
-                sitepackages.append(
-                        os.path.join("/Library", framework,
-                            '%d.%d' % sys.version_info[:2], "site-packages"))
+        # for framework builds *only* we add the standard Apple
+        # locations.
+        if sys.platform == "darwin" and sys._framework:
+            sitepackages.append(
+                    os.path.join("/Library", sys._framework,
+                        '%d.%d' % sys.version_info[:2], "site-packages"))
     return sitepackages
 
 def addsitepackages(known_paths, prefixes=None):

With this additionnal change, the speedup on macOS is quite significant: -13.4 ms (1.61x faster)!

macbook:master haypo$ ./python.exe -m perf command --inherit=PYTHONPATH -v -o ref.json -- ./python.exe -c pass 
...
macbook:master haypo$ ./python.exe -m perf command --inherit=PYTHONPATH -v -o pr136.json -- ./python.exe -c pass
...
macbook:master haypo$ ./python.exe -m perf compare_to ref.json pr136.json 
Mean +- std dev: [ref] 35.4 ms +- 1.7 ms -> [pr136] 22.0 ms +- 1.9 ms: 1.61x faster (-38%)

cc @1st1: Yury, since you use macOS, you probably want to see this change merged :-)

vstinner

New review.

vstinner · 2017-06-28T12:13:56Z

Lib/site.py

@@ -234,6 +234,27 @@ def check_enableusersite():

    return True

+
+def _getuserbase():
+    # Stripped version of sysconfig._getuserbase()


Can you please elaborate this comment? Please explain that the function was duplicated to speedup Python startup and avoid the sysconfig import in the site module. Add a reference to the bpo 29585.

It seems like you modified this change: you moved "if env_base: return env_base" at the top. Please modify also sysconfig._getuserbase(). I would also prefer the sysconfig also uses sys._framework, instead of get_config_var("PYTHONFRAMEWORK").

vstinner · 2017-06-28T12:14:46Z

Lib/site.py

+
+def _get_path(userbase):
+    # stripped version of sysconfig.get_path('purelib', os.name + '_user')
+    version = sys.version_info[:2]


Is [:2] really needed?

vstinner · 2017-06-28T12:15:38Z

Lib/site.py

    return USER_BASE

+
+def _get_path(userbase):
+    # stripped version of sysconfig.get_path('purelib', os.name + '_user')


Again, please elaborate the comment. (Maybe point to _getuserbase() above?)

vstinner · 2017-06-28T12:19:10Z

Ned Deily: "This is a major change: by duplicating code from sysconfig.py into site.py, there would now be a implicit link between the two and a potential maintenance problem. If we were to do this, there should at least be mention of these duplications in sysconfig.py and/or tests for the duplicated behavior."

@methane added a comment in sysconfig.py, IMHO it's enough.

About testing: @methane, can you try to write a test to check that site and sysconfig return the same value for the two private functions? You may have to tag the unit test with @cpython_only, since the two new site functions are private.

Ned Deily: "Also, there should be a b.p.o issue for this proposed change."

Done.

Since most Ned's requests are done, I dismiss his review.

@methade modified this PR.

vstinner · 2017-06-28T12:20:36Z

Except of my minor commits, I now like the overall shape of the PR. I now agree that writing a new _site module is not necessary, it's better to keep all code in the site.py file.

methane · 2017-06-28T12:27:17Z

On macOS, performance gain is more impressive than Linux:

$ ./python.exe -m perf compare_to ref.json pr136.json
Mean +- std dev: [ref] 30.4 ms +- 0.9 ms -> [pr136] 21.6 ms +- 4.0 ms: 1.40x faster (-29%)

methane · 2017-06-28T12:28:13Z

Oh, I'm sorry. I missed your above comment.

vstinner

LGTM, but I would like to see an answer to the question on tests (check that site functions return the same result than sysconfig?) before approving your change :-)

vstinner

Thanks! The latest change is much better than the first iteration, and it now LGTM!

methane · 2017-06-28T15:32:02Z

Thank you for review.

1st1 · 2017-06-28T15:33:28Z

Why couldn't we make 'sysconfig' to import a couple of private functions from 'site', why copying?

vstinner · 2017-06-28T15:34:38Z

Why couldn't we make 'sysconfig' to import a couple of private functions from 'site', why copying?

It's not a pure copy/paste, one function is specialized for site use case. I dislike the idea of importing site code from sysconfig.

vstinner · 2017-06-28T15:34:52Z

Thank you for review.

Thanks for this cool and cheap speedup!

methane · 2017-06-28T15:37:11Z

Ah, I've missed build failure on Windows.

vstinner · 2017-06-28T15:37:22Z

Oh, Windows build is broken :-(

     2>..\Python\sysmodule.c(1968): error C2065: 'PYTHONFRAMEWORK': undeclared identifier [C:\buildbot.python.org\3.x.kloth-win64\build\PCbuild\pythoncore.vcxproj]

Need to fix PC/pyconfig.h?

vstinner · 2017-06-28T16:04:59Z

Oh, AppVeyor didn't catch the bug simply because it wasn't run on this PR!

vstinner · 2017-06-28T16:05:23Z

@methane proposed PR #2476 to fix PC/pyconfig.h, I proposed PR #2477.

This reverts commit a8f8d5b.

Python 3 has no xrange.

the-knights-who-say-ni added the CLA signed label Feb 16, 2017

methane force-pushed the optimize-site-startup branch from 4f58b0c to 483769e Compare February 16, 2017 14:35

ned-deily previously requested changes Feb 16, 2017

View reviewed changes

methane changed the title ~~optimize site.py~~ [RFC] bpo-29585: optimize site.py startup time Feb 17, 2017

vstinner requested changes Feb 17, 2017

View reviewed changes

methane force-pushed the optimize-site-startup branch from dfeced1 to 45f5e9a Compare February 18, 2017 05:04

methane changed the title ~~[RFC] bpo-29585: optimize site.py startup time~~ [RFC] [bpo-29585](https://bugs.python.org/issue29585): optimize site.py startup time Feb 18, 2017

methane changed the title ~~[RFC] [bpo-29585](https://bugs.python.org/issue29585): optimize site.py startup time~~ [RFC] bpo-29585: optimize site.py startup time Feb 18, 2017

methane force-pushed the optimize-site-startup branch from 45f5e9a to 07369f1 Compare February 18, 2017 05:10

methane changed the title ~~[RFC] bpo-29585: optimize site.py startup time~~ bpo-29585: optimize site.py startup time Feb 19, 2017

methane added 3 commits June 28, 2017 19:28

Add PYTHONFRAMEWORK to pyconfig.h

62388c4

Add sys._framework

2e12d42

optimize site.py

ff0d05c

Skip importing sysconfig when possible. Median +- std dev: [default] 15.8 ms +- 0.0 ms -> [patched] 14.7 ms +- 0.0 ms: 1.07x faster (-7%)

methane force-pushed the optimize-site-startup branch from 75b4c6c to ff0d05c Compare June 28, 2017 10:29

run autoreconf

a1976a7

vstinner added the performance Performance or resource usage label Jun 28, 2017

Add note comment for duplicated code.

5aabcec

vstinner requested changes Jun 28, 2017

View reviewed changes

remove sysconfig import on darwin platform

6f78303

More elaborate comments about copied functions.

33c420a

vstinner reviewed Jun 28, 2017

View reviewed changes

methane added 2 commits June 29, 2017 00:14

add test for copied two functions

bdabfd0

Add NEWS entry

4846349

vstinner approved these changes Jun 28, 2017

View reviewed changes

methane merged commit a8f8d5b into python:master Jun 28, 2017

methane deleted the optimize-site-startup branch June 28, 2017 15:31

vstinner mentioned this pull request Jun 28, 2017

bpo-29585: Define PYTHONFRAMEWORK in PC/pyconfig.h #2477

Merged

vstinner added a commit that referenced this pull request Jun 28, 2017

Revert "bpo-29585: optimize site.py startup time (GH-136)"

1365b10

This reverts commit a8f8d5b.

vstinner mentioned this pull request Jun 28, 2017

Revert "bpo-29585: optimize site.py startup time" #2478

Closed

methane mentioned this pull request Jun 29, 2017

bpo-29585: Fix sysconfig.get_config_var("PYTHONFRAMEWORK") #2483

Merged

akruis pushed a commit to akruis/cpython that referenced this pull request Oct 10, 2017

Fix Issue python#136: replace xrange with range

1f9f0ba

Python 3 has no xrange.

akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017

Stackless issues python#135, python#136: update changelog.txt

f845a0f

morphex mentioned this pull request Jul 24, 2022

Python crashes on large number of client connects to socketserver #95204

Closed

gvanrossum mentioned this pull request Aug 22, 2023

heap-use-after-free in _PyFunction_LookupByVersion #108253

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-29585: optimize site.py startup time #136

bpo-29585: optimize site.py startup time #136

methane commented Feb 16, 2017 •

edited

Loading

methane commented Feb 16, 2017

ned-deily left a comment •

edited

Loading

vstinner left a comment

methane commented Feb 18, 2017 •

edited

Loading

methane commented Jun 28, 2017

methane commented Jun 28, 2017

vstinner commented Jun 28, 2017 •

edited

Loading

vstinner commented Jun 28, 2017

vstinner left a comment

vstinner Jun 28, 2017

vstinner Jun 28, 2017

vstinner Jun 28, 2017

vstinner commented Jun 28, 2017

vstinner commented Jun 28, 2017

methane commented Jun 28, 2017

methane commented Jun 28, 2017

vstinner left a comment

vstinner left a comment

methane commented Jun 28, 2017

1st1 commented Jun 28, 2017

vstinner commented Jun 28, 2017

vstinner commented Jun 28, 2017

methane commented Jun 28, 2017

vstinner commented Jun 28, 2017

vstinner commented Jun 28, 2017

vstinner commented Jun 28, 2017

bpo-29585: optimize site.py startup time #136

bpo-29585: optimize site.py startup time #136

Conversation

methane commented Feb 16, 2017 • edited Loading

methane commented Feb 16, 2017

ned-deily left a comment • edited Loading

Choose a reason for hiding this comment

vstinner left a comment

Choose a reason for hiding this comment

methane commented Feb 18, 2017 • edited Loading

methane commented Jun 28, 2017

methane commented Jun 28, 2017

vstinner commented Jun 28, 2017 • edited Loading

vstinner commented Jun 28, 2017

vstinner left a comment

Choose a reason for hiding this comment

vstinner Jun 28, 2017

Choose a reason for hiding this comment

vstinner Jun 28, 2017

Choose a reason for hiding this comment

vstinner Jun 28, 2017

Choose a reason for hiding this comment

vstinner commented Jun 28, 2017

vstinner commented Jun 28, 2017

methane commented Jun 28, 2017

methane commented Jun 28, 2017

vstinner left a comment

Choose a reason for hiding this comment

vstinner left a comment

Choose a reason for hiding this comment

methane commented Jun 28, 2017

1st1 commented Jun 28, 2017

vstinner commented Jun 28, 2017

vstinner commented Jun 28, 2017

methane commented Jun 28, 2017

vstinner commented Jun 28, 2017

vstinner commented Jun 28, 2017

vstinner commented Jun 28, 2017

methane commented Feb 16, 2017 •

edited

Loading

ned-deily left a comment •

edited

Loading

methane commented Feb 18, 2017 •

edited

Loading

vstinner commented Jun 28, 2017 •

edited

Loading