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

operator.py: move the Python implementation in the else block of try/except ImportError #63428

Closed
vstinner opened this issue Oct 11, 2013 · 10 comments
Labels
performance Performance or resource usage

Comments

@vstinner
Copy link
Member

BPO 19229
Nosy @pitrou, @vstinner, @tiran, @skrah, @zware
Files
  • operator.py
  • builtin_operator.patch
  • builtin_operator_diff.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-12-16.23:19:57.352>
    created_at = <Date 2013-10-11.23:49:11.908>
    labels = ['performance']
    title = 'operator.py: move the Python implementation in the else block of try/except ImportError'
    updated_at = <Date 2013-12-16.23:19:57.352>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2013-12-16.23:19:57.352>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-12-16.23:19:57.352>
    closer = 'vstinner'
    components = []
    creation = <Date 2013-10-11.23:49:11.908>
    creator = 'vstinner'
    dependencies = []
    files = ['32053', '32403', '32404']
    hgrepos = []
    issue_num = 19229
    keywords = ['patch']
    message_count = 10.0
    messages = ['199525', '199566', '199701', '201583', '201585', '201638', '201639', '201640', '201641', '206022']
    nosy_count = 6.0
    nosy_names = ['pitrou', 'vstinner', 'christian.heimes', 'Arfrever', 'skrah', 'zach.ware']
    pr_nums = []
    priority = 'normal'
    resolution = 'wont fix'
    stage = None
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue19229'
    versions = ['Python 3.4']

    @vstinner
    Copy link
    Member Author

    To speedup Python startup, it may be interesting to not create useless many functions and classes in operator.py: "from _operator import *" will remove them a few line later.

    What do you think of moving the Python implementation of the operator module inside in the else block of "try/except ImportError" section?

    See attached operator.py for an example.

    It adds an ugly level of indentation, but it's for performances!

    Another option is to add a _pyoperator module.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Oct 12, 2013

    Using the microbenchmark I get (standard version):

    ./python -m timeit "import sys; modname='operator'" "__import__(modname); del sys.modules[modname]"
    1000 loops, best of 3: 460 usec per loop

    Victor's version:

    ./python -m timeit "import sys; modname='operator'" "__import__(modname); del sys.modules[modname]"
    1000 loops, best of 3: 355 usec per loop

    Importing _operator directly:

    ./python -m timeit "import sys; modname='_operator'" "__import__(modname); del sys.modules[modname]"
    10000 loops, best of 3: 35.7 usec per loop

    Extrapolating from what I did with decimal, I guess a _pyoperator
    version could get down to something like 70 usec.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Oct 13, 2013

    To be fair, for the startup time I can't really detect any difference between importing _operator directly and the current setup.

    @vstinner
    Copy link
    Member Author

    Another option is to add a _pyoperator module.

    Attached builtin_operator.patch patch implements this option: operator.c becomes the main operator module, _pyoperator is the pure Python implementation (don't use "from _operator import *" anymore).

    With the patch:

    $ ./python -m timeit "import sys; modname='_pyoperator'" "__import__(modname); del sys.modules[modname]" 
    1000 loops, best of 3: 276 usec per loop
    
    $ ./python -m timeit "import sys; modname='operator'" "__import__(modname); del sys.modules[modname]" 
    10000 loops, best of 3: 22.7 usec per loop

    The patch is huge because files are renamed: see builtin_operator_diff.patch for the diff.

    @vstinner
    Copy link
    Member Author

    Without the patch:

    $ ./python -m timeit "import sys; modname='operator'" "__import__(modname); del sys.modules[modname]" 
    1000 loops, best of 3: 289 usec per loop
    
    $ ./python -m timeit "import sys; modname='_operator'" "__import__(modname); del sys.modules[modname]" 
    10000 loops, best of 3: 21.4 usec per loop

    "import operator" is 12.7x faster (289 usec => 22.7 usec) with builtin_operator.patch.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 29, 2013

    Why not:

    try:
    from _operator import *
    except ImportError:
    from _pyoperator import *

    @vstinner
    Copy link
    Member Author

    Why not:

    try:
    from _operator import *
    except ImportError:
    from _pyoperator import *

    Let's try (I replaced operator.py with these 4 lines).

    $ ./python -m timeit "import sys; modname='operator'" "__import__(modname); del sys.modules[modname]; del sys.modules['_operator']" 
    10000 loops, best of 3: 165 usec per loop
    
    $ ./python -m timeit "import sys; modname='operator'" "__import__(modname); del sys.modules[modname]" 
    10000 loops, best of 3: 136 usec per loop

    "import operator" is only 2x faster (289 usec => 136 usec). It's less interesting. And what would be the purpose of a file of 4 line which containing "import *"? Do you think that PyPy, IronPython and Jython will reuse such trampoline/wrapper?

    @vstinner vstinner added the performance Performance or resource usage label Oct 29, 2013
    @pitrou
    Copy link
    Member

    pitrou commented Oct 29, 2013

    "import operator" is only 2x faster (289 usec => 136 usec). It's less
    interesting.

    The real question for me is: why are you interested in speeding up the
    import of the operator module? 200 µs won't make a visible difference.

    And what would be the purpose of a file of 4 line which
    containing "import *"?

    To make it obvious that there are two implementations, one of which is
    a fallback.

    @pitrou pitrou changed the title operator.py: move the Python implementation in the else block of try/except ImportError operator.py: move the Python implementation in the else block of try/except ImportError Oct 29, 2013
    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Oct 29, 2013

    I understand the desire to speed things up, but either the four-line facade
    module or else a single module is probably required by PEP-399.

    @skrah skrah mannequin changed the title operator.py: move the Python implementation in the else block of try/except ImportError operator.py: move the Python implementation in the else block of try/except ImportError Oct 29, 2013
    @vstinner
    Copy link
    Member Author

    "The real question for me is: why are you interested in speeding up the
    import of the operator module? 200 µs won't make a visible difference."

    Alone, the gain is useless, but it's like the work done in Python 3.4 to avoid loading some modules at startup. The overall idea is to have a fast startup time.

    I heard that Python 3 startup time is a major blocker point for Mercurial for example.

    But maybe this specific issue is not worth the trouble. (Other parts should be optimized.)

    @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
    performance Performance or resource usage
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants