Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't import re and sysconfig in site.py #63404

Closed
tiran opened this issue Oct 9, 2013 · 15 comments
Closed

Don't import re and sysconfig in site.py #63404

tiran opened this issue Oct 9, 2013 · 15 comments
Labels
OS-mac performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@tiran
Copy link
Member

tiran commented Oct 9, 2013

BPO 19205
Nosy @warsaw, @pitrou, @vstinner, @tiran
Files
  • startup_no_re.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2013-10-21.09:09:25.888>
    created_at = <Date 2013-10-09.13:15:01.526>
    labels = ['OS-mac', 'library', 'performance']
    title = "Don't import re and sysconfig in site.py"
    updated_at = <Date 2013-10-21.09:09:25.886>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2013-10-21.09:09:25.886>
    actor = 'christian.heimes'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-10-21.09:09:25.888>
    closer = 'christian.heimes'
    components = ['Library (Lib)', 'macOS']
    creation = <Date 2013-10-09.13:15:01.526>
    creator = 'christian.heimes'
    dependencies = []
    files = ['32048']
    hgrepos = []
    issue_num = 19205
    keywords = ['patch']
    message_count = 15.0
    messages = ['199295', '199330', '199502', '199503', '199506', '199514', '199515', '199516', '199543', '199545', '199563', '199565', '200721', '200731', '200734']
    nosy_count = 5.0
    nosy_names = ['barry', 'pitrou', 'vstinner', 'christian.heimes', 'python-dev']
    pr_nums = []
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue19205'
    versions = ['Python 3.4']

    @tiran
    Copy link
    Member Author

    tiran commented Oct 9, 2013

    The site module is loaded at every startup. Nowadays it imports the re and the sysconfig modules. The re module is used for venv config parsing and inside sysconfig. sysconfig is loaded to find the location of the user's site-packages directory.

    Suggestions:

    • Only import re and compile CONFIG_LINE when a venv config file is found

    • Don't rely on sysconfig for user's site-packages directory. Instead the sysconfig module should rely on site.py to get its location.

    Without re and sysconfig Python would import only 45 instead of 56 modules at startup (tested on Linux).

    @tiran tiran added interpreter-core (Objects, Python, Grammar, and Parser dirs) stdlib Python modules in the Lib dir labels Oct 9, 2013
    @tiran tiran changed the title Don Don't import re and sysconfig in site.py Oct 9, 2013
    @tiran tiran added the performance Performance or resource usage label Oct 9, 2013
    @tiran
    Copy link
    Member Author

    tiran commented Oct 9, 2013

    The site and sysconfig module are too intermingled to remove the import of sysconfig from the site module. However the sysconfig module doesn't use the re module in most cases anymore. The parsing functions for Makefile and pyconfig.h are only used by distutils.

    The patch moves "import re" inside three functions.

    @tiran
    Copy link
    Member Author

    tiran commented Oct 11, 2013

    Here is a new patch with unit test and patch for the locale module. The locale modules is loaded by the _io module when any standard stream is not connected to a terminal (or so).

    @pitrou
    Copy link
    Member

    pitrou commented Oct 11, 2013

    Review posted on Rietveld.

    @vstinner
    Copy link
    Member

    I accept hacks to speedup Python is the site module, but it becomes more surprising in the locale module. The issue bpo-9548 proposes to a more generic solution for the locale module at startup.

    -CONFIG_LINE = re.compile(r'^(?P<key>(\w|[-_])+)\s*=\s*(?P<value>.*)\s*$')
    +CONFIG_LINE = None

    If you set the constant to None, it's better to remove it completly (or make it private). It's a public variable, someone may try to read it. I don't know why it is public.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 11, 2013

    New changeset 406529adf156 by Christian Heimes in branch 'default':
    Issue bpo-19205: Don't import the 're' module in site and sysconfig module to
    http://hg.python.org/cpython/rev/406529adf156

    @tiran
    Copy link
    Member Author

    tiran commented Oct 11, 2013

    Thanks for your input!

    @tiran tiran closed this as completed Oct 11, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 11, 2013

    New changeset 2cd1b28d1666 by Christian Heimes in branch 'default':
    Issue bpo-19205 fix 406529adf156
    http://hg.python.org/cpython/rev/2cd1b28d1666

    @pitrou
    Copy link
    Member

    pitrou commented Oct 12, 2013

    Christian, the test is failing on Snow Leopard:

    ======================================================================
    FAIL: test_startup_imports (test.test_site.StartupImportTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_site.py", line 435, in test_startup_imports
        self.assertFalse(modules.intersection(re_mods))
    AssertionError: {'re', 'sre_compile', 'sre_constants', 'sre_parse', '_sre'} is not false

    http://buildbot.python.org/all/builders/AMD64%20Snow%20Leop%203.x/builds/106/steps/test/logs/stdio

    @pitrou pitrou reopened this Oct 12, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 12, 2013

    New changeset a57dfbba91f9 by Christian Heimes in branch 'default':
    Issue bpo-19205: add debugging output for failing test on Snow Leopard
    http://hg.python.org/cpython/rev/a57dfbba91f9

    @vstinner
    Copy link
    Member

    New changeset a57dfbba91f9 by Christian Heimes in branch 'default':
    Issue bpo-19205: add debugging output for failing test on Snow Leopard
    http://hg.python.org/cpython/rev/a57dfbba91f9

    So the "import re" comes from _osx_support, _osx_support is imported by sysconfig.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 12, 2013

    New changeset 9f6ef09f6492 by Christian Heimes in branch 'default':
    Issue bpo-19205: _osx_support uses the re module all over the place. Omit the test for nw.
    http://hg.python.org/cpython/rev/9f6ef09f6492

    @tiran
    Copy link
    Member Author

    tiran commented Oct 21, 2013

    The imports in _osx_support counteract the performance boost. I neither have an OS X machine and nor do I understand the internals of _osx_support. Perhaps somebody else likes to work on the module.

    @tiran tiran added OS-mac and removed interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Oct 21, 2013
    @tiran tiran removed their assignment Oct 21, 2013
    @vstinner
    Copy link
    Member

    The imports in _osx_support counteract the performance boost. I neither have an OS X machine and nor do I understand the internals of _osx_support. Perhaps somebody else likes to work on the module.

    Please open a separated issue for OS X, this issue can be closed IMO.

    @tiran
    Copy link
    Member Author

    tiran commented Oct 21, 2013

    Good point, bpo-19325

    @tiran tiran closed this as completed Oct 21, 2013
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    OS-mac performance Performance or resource usage stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants