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

Faster positional arguments parsing in PyArg_ParseTupleAndKeywords #73215

Closed
serhiy-storchaka opened this issue Dec 20, 2016 · 8 comments
Closed
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@serhiy-storchaka
Copy link
Member

BPO 29029
Nosy @vstinner, @methane, @serhiy-storchaka
Files
  • PyArg_ParseTupleAndKeywords-faster-positional-args-parse.patch
  • PyArg_ParseTupleAndKeywords-faster-positional-args-parse-2.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 2017-01-17.08:15:00.665>
    created_at = <Date 2016-12-20.21:30:18.353>
    labels = ['interpreter-core', '3.7', 'performance']
    title = 'Faster positional arguments parsing in PyArg_ParseTupleAndKeywords'
    updated_at = <Date 2017-01-17.08:15:00.664>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-01-17.08:15:00.664>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-01-17.08:15:00.665>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2016-12-20.21:30:18.353>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['45977', '46307']
    hgrepos = []
    issue_num = 29029
    keywords = ['patch']
    message_count = 8.0
    messages = ['283711', '285515', '285590', '285591', '285592', '285617', '285621', '285622']
    nosy_count = 4.0
    nosy_names = ['vstinner', 'methane', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue29029'
    versions = ['Python 3.7']

    @serhiy-storchaka
    Copy link
    Member Author

    Proposed patch speeds up parsing positional arguments in PyArg_ParseTupleAndKeywords().

    $ ./python -m perf timeit "1 .to_bytes(1, 'little', signed=False)"
    Unpatched:  Median +- std dev: 2.01 us +- 0.09 us
    Patched:    Median +- std dev: 1.23 us +- 0.03 us

    Currently names of all passed positional argument are looked up in the kwargs dictionary for checking if the argument is passed as positional and keyword argument. With the patch this is checked only if there are unhandled keyword arguments after parsing keyword arguments that don't passed as positional arguments.

    The same optimization also is applied to _PyArg_ParseTupleAndKeywordsFast() and like, but the effect is much smaller.

    The patch also includes tiny refactoring.

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Dec 20, 2016
    @methane
    Copy link
    Member

    methane commented Jan 15, 2017

    LGTM

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 16, 2017

    New changeset a36e60c5fee0 by Victor Stinner in branch 'default':
    Rename keywords to kwargs in getargs.c
    https://hg.python.org/cpython/rev/a36e60c5fee0

    @vstinner
    Copy link
    Member

    Oh, I missed this issue. Since I made minor cleanup recently, I took the liberty of rebasing your patch.

    I also pushed your "keywords => kwargs" change, just to make the patch simpler to review.

    @vstinner
    Copy link
    Member

    Unpatched: Median +- std dev: 2.01 us +- 0.09 us
    Patched: Median +- std dev: 1.23 us +- 0.03 us

    Oh wow, impressive speedup! As usual, good job Serhiy ;-)

    PyArg_ParseTupleAndKeywords-faster-positional-args-parse-2.patch: LGTM! Can you please push it?

    @methane
    Copy link
    Member

    methane commented Jan 17, 2017

    LGTM again.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 17, 2017

    New changeset a8563ef0eb8a by Serhiy Storchaka in branch 'default':
    Issue bpo-29029: Speed up processing positional arguments in
    https://hg.python.org/cpython/rev/a8563ef0eb8a

    @serhiy-storchaka
    Copy link
    Member Author

    Thank you for your reviews Inada and Victor. Thank you for rebasing the patch Victor.

    I were going first to try an alternative patch, maybe less efficient, but allowing to share more code. But since the code is changed so fast, it is better to commit the ready patch.

    @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
    3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants