Skip to content

Python startup performance regression #72823

Closed

Description

@vstinner
BPO 28637
Nosy @rhettinger, @terryjreedy, @gpshead, @vsajip, @vstinner, @tiran, @ned-deily, @ethanfurman, @serhiy-storchaka, @wm75
Files
  • site-not-use-re.patch
  • no_use_re_in_site_py.patch
  • site-not-use-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 2016-11-14.11:55:02.680>
    created_at = <Date 2016-11-07.23:08:21.740>
    labels = ['3.7', 'release-blocker', 'performance']
    title = 'Python startup performance regression'
    updated_at = <Date 2017-02-17.15:37:46.818>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2017-02-17.15:37:46.818>
    actor = 'gvanrossum'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-11-14.11:55:02.680>
    closer = 'vstinner'
    components = []
    creation = <Date 2016-11-07.23:08:21.740>
    creator = 'vstinner'
    dependencies = []
    files = ['45389', '45390', '45391']
    hgrepos = []
    issue_num = 28637
    keywords = ['patch']
    message_count = 38.0
    messages = ['280256', '280258', '280259', '280260', '280261', '280262', '280266', '280267', '280268', '280269', '280271', '280272', '280273', '280274', '280296', '280301', '280302', '280304', '280305', '280306', '280312', '280328', '280346', '280353', '280362', '280370', '280378', '280380', '280383', '280409', '280552', '280558', '280559', '280629', '280636', '280753', '280758', '280760']
    nosy_count = 11.0
    nosy_names = ['rhettinger', 'terry.reedy', 'gregory.p.smith', 'vinay.sajip', 'vstinner', 'christian.heimes', 'ned.deily', 'ethan.furman', 'python-dev', 'serhiy.storchaka', 'wolma']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue28637'
    versions = ['Python 3.6', 'Python 3.7']

    Activity

    1. vstinner commented on Nov 7, 2016

      @vstinner
      MemberAuthor

      The changeset 223731925d06 of the issue issue bpo-28082 "use IntFlag for re constants" made Python startup 34% slower:

      • rev 5637c9b4dd4c: Median +- std dev: 19.5 ms +- 0.0 ms
      • rev 223731925d06: Median +- std dev: 26.2 ms +- 0.0 ms

      The change adds "import enum" in Lib/re.py, so the enum module is imported, whereas it wasn't before. It seems like importing enum takes 6.7 ms.

      I propose to revert the change 223731925d06 in Python 3.6 and reopen the issue bpo-28082 to try to find another solution for the re module which doesn't impact Python startup performance.

    2. gvanrossum commented on Nov 7, 2016

      @gvanrossum
      Member

      Reverting is reasonable, though it may break some code that started using this in 3.6b1. Such is the life of beta testers. The benefit of using Enum for re constants doesn't seem worth such a big increase in startup time.

    3. ethanfurman commented on Nov 7, 2016

      @ethanfurman
      Member

      Ouch, that's a lot slower!

      Victor, can you add the commands you are using for timing so I can work on making enum imports faster?

    4. vstinner commented on Nov 7, 2016

      @vstinner
      MemberAuthor

      Ethan Furman added the comment:

      Victor, can you add the commands you are using for timing so I can work on making enum imports faster?

      Results of the python_startup benchmark:
      https://speed.python.org/timeline/#/?exe=4&ben=python_startup&env=1&revs=50&equid=off&quarts=on&extr=on

      This website uses the following benchmark suite:
      https://github.com/python/performance

      You can run the benchmark using:
      python3 -m performance run -b python_startup

    5. vstinner commented on Nov 7, 2016

      @vstinner
      MemberAuthor

      Oh, I understood why I had issues to reproduce the startup performance slowdown. When Python is run from the source code using ./python, the re module is not imported. When Python is installed, the re is not imported by default neither:

      haypo@speed-python$ prefix/bin/python3 -c 'import sys; print("re" in sys.modules)'
      False

      BUT when Python is started from a virtual environment (created by the "venv" module), the re module is important by default.

      haypo@speed-python$ venv/bin/python3 -c 'import sys; print("re" in sys.modules)'
      True

      If the site module is not imported, the re module is not imported:

      haypo@speed-python$ venv/bin/python3 -S -c 'import sys; print("re" in sys.modules)'
      False

      The /home/haypo/benchmarks/prefix/lib/python3.6/site.py file is generated by the venv module and contains:

      def venv(...):
          ...
          if candidate_confs:
              import re
              config_line = re.compile(CONFIG_LINE)
              ...
    6. python-dev commented on Nov 7, 2016

      python-devmannequin
      Mannequin

      New changeset d903a243c281 by Victor Stinner in branch '3.6':
      Issue bpo-28637: Revert issue bpo-28082, don't import enum in re
      https://hg.python.org/cpython/rev/d903a243c281

    7. ned-deily commented on Nov 8, 2016

      @ned-deily
      Member

      With being this close to release and with two weeks to go until the final beta, it seems a bit premature to immediately revert this without exploring all the alternatives especially since it seems the effects are limited to venvs. In any case, we do need to have a final answer well before the b4 cutoff (2016-11-21). Possible options:

      1. leave the re contents reverted (current status)
      2. find a way to mitigate the performance impact of importing re and enum (perhaps making them builtins in Setup.dist?)
      3. avoid the use of re in site.py venv
      4. others?
    8. vstinner commented on Nov 8, 2016

      @vstinner
      MemberAuthor

      Possible options:

      1. avoid the use of re in site.py venv

      That would be a tiny but nice enhancement.

      re is a commonly used module. I would prefer to not make its import time slower. Python 3 startup is already something like 3x slower than Python 2. Please don't make it even slower :-( We worked on optimizing Python 3 startup time.

      1. find a way to mitigate the performance impact of importing re and enum (perhaps making them builtins in Setup.dist?)

      I reverted the change, because Guido and Ethan seem to be in favor of a revert, and I don't expect any simple solution for this issue.

      I'm quite sure that importing enum.py is slow. Optimizing enum.py import time is a large task.

      Another option to explore is to delay the creation/instanciation of the re flags, something like lazy module import... but at the module attribute level :-)

    9. vstinner commented on Nov 8, 2016

      @vstinner
      MemberAuthor

      I remove the release blocker flag since the change was reverted.

    10. ned-deily commented on Nov 8, 2016

      @ned-deily
      Member

      Please leave this as a "release blocker" until we have had a chance to explore all the options for 3.6.0b4. We may still very well decide that reverting is the least bad option but I don't want to rule out other options without some further investigation. We have time and the reverting could be a fairly significant change.

    11. 17 remaining items

    12. changed the title [-]Python startup performance regression due to enum in re[/-] [+]Python startup performance regression[/+] on Nov 9, 2016
    13. gpshead commented on Nov 9, 2016

      @gpshead
      Member

      re: Ethan's question - I think the enum use should be restored in re.

      I realize bpo-28082 (yay palindrome number) is not an urgent change but we created IntEnum for the purpose of more identifiable integer constants.

      So a microbenchmark of "import re" slows down. so what? I don't find this to be a big deal. Other standard library modules also use enum and I expect more to do so in the future. In realistic size programs other things use enum as well so there isn't a hit.

      PS thanks for the site.py improvements!

    14. serhiy-storchaka commented on Nov 9, 2016

      @serhiy-storchaka
      Member

      On my computer:

      Importing empty module: 160 us
      Creating empty class: 30 us
      Creating empty function: 0.16 us
      Creating empty Enum/IntEnum: 125/150 us
      Creating Enum/IntEnum member: 25/27 us
      Creating empty namedtuple: 600 us
      Creating namedtuple member: 50 us
      Importing the itertools module: 40 us
      Importing the io module: 900 us
      Importing the os module: 1600 us
      Importing the functools module: 2100 us
      Importing the re module (with all sre submodules): 3300 us
      Python startup time: 43000 us

    15. vstinner commented on Nov 9, 2016

      @vstinner
      MemberAuthor

      About the re module, there is still a regression on the performance of
      the import time: see my msg280271.

      We may also revert the changeset 88110cfbf4dc (issue bpo-28193) if we
      want to keep "import re" as fast as Python 3.5.

      The question is if we consider that the re module is part of the
      Python core modules which must be blasing fast to import.

    16. gvanrossum commented on Nov 9, 2016

      @gvanrossum
      Member

      Honestly now the basic startup time is under control I am fine with
      restoring the re enum. We can optimize that later.

      --Guido (mobile)

    17. vstinner commented on Nov 10, 2016

      @vstinner
      MemberAuthor

      Oh nice, python_startup benchmark at Nov 10 (rev at 8ebaa546a033) is now faster than python_startup at Feb 29 (rev dfeadfb4c8cc): new 16.8 ms, old 19.5. And it's much before when I opened the issue: 26.6 ms. Thanks.

      https://speed.python.org/timeline/#/?exe=4&ben=python_startup&env=1&revs=50&equid=off&quarts=on&extr=on

      The main difference is the removal of "import re" from Lib/site.py. If I added back "import re", python_startup comes back to 24.2 ms +- 0.0 ms. I understand that "import re" adds 7.4 ms. Any additional import has an important cost of python startup performance :-/

    18. ethanfurman commented on Nov 11, 2016

      @ethanfurman
      Member

      Leaving re out of site.py is fine; the current question is whether to add enum back to re.

    19. gvanrossum commented on Nov 11, 2016

      @gvanrossum
      Member

      I think it's okay to add enum back into re.

    20. rhettinger commented on Nov 12, 2016

      @rhettinger
      Contributor

      Serhiy, thanks for your work. By isolating the few easily rewritten lines in site.py, you saved us from endless contortions trying to avoid using our own batteries.

    21. serhiy-storchaka commented on Nov 12, 2016

      @serhiy-storchaka
      Member

      Thanks to Wolfgang. This is his idea, I just faster wrote the patch.

    22. python-dev commented on Nov 14, 2016

      python-devmannequin
      Mannequin

      New changeset 5fd69d4a93e0 by Victor Stinner in branch '3.6':
      Issue bpo-28637: Reapply changeset 223731925d06
      https://hg.python.org/cpython/rev/5fd69d4a93e0

    23. vstinner commented on Nov 14, 2016

      @vstinner
      MemberAuthor

      Ethan Furman: "Does this mean we can put enum back in re?"

      I checked manually: adding "import enum" to Lib/re.py has no more impact on "python_startup" benchmark.

      Guido van Rossum: "Honestly now the basic startup time is under control I am fine with restoring the re enum. We can optimize that later."

      I concur with Guido, so I reapplied Ethan's changeset. Enjoy readable str() on re flags ;-)

    24. vstinner commented on Nov 14, 2016

      @vstinner
      MemberAuthor

      The initial bug (performance regression in python_startup) is fixed. Thanks everyone for the nice discussion. I close the issue.

      Please open new issues if you want to continue efforts on making CPython faster ;-)

    25. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions