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

Argument Clinic: improve generated parser for 1-argument functions #67680

Closed
serhiy-storchaka opened this issue Feb 20, 2015 · 11 comments
Closed
Assignees
Labels
build The build process and cross-build topic-argument-clinic type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 23492
Nosy @pitrou, @scoder, @larryhastings, @serhiy-storchaka, @MojoVampire
Dependencies
  • bpo-23501: Argument Clinic: generate code into separate files by default
  • Files
  • clinic_meth_o.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 = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2015-04-03.22:00:38.147>
    created_at = <Date 2015-02-20.14:40:11.906>
    labels = ['type-feature', 'expert-argument-clinic', 'build']
    title = 'Argument Clinic: improve generated parser for 1-argument functions'
    updated_at = <Date 2015-04-04.14:07:58.523>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2015-04-04.14:07:58.523>
    actor = 'python-dev'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2015-04-03.22:00:38.147>
    closer = 'serhiy.storchaka'
    components = ['Build', 'Argument Clinic']
    creation = <Date 2015-02-20.14:40:11.906>
    creator = 'serhiy.storchaka'
    dependencies = ['23501']
    files = ['38188']
    hgrepos = []
    issue_num = 23492
    keywords = ['patch']
    message_count = 11.0
    messages = ['236288', '236313', '236315', '236367', '236372', '236382', '236385', '236388', '236408', '240038', '240072']
    nosy_count = 6.0
    nosy_names = ['pitrou', 'scoder', 'larry', 'python-dev', 'serhiy.storchaka', 'josh.r']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue23492'
    versions = ['Python 3.5']

    @serhiy-storchaka
    Copy link
    Member Author

    Proposed patch improve generated parsers for functions with single positional argument. Now they always generated as METH_O and PyArg_Parse() is used to parse single argument.

    To avoid code churn in this and following changes it would be worth to extract all generated code in separated files.

    @serhiy-storchaka serhiy-storchaka added build The build process and cross-build type-feature A feature request or enhancement labels Feb 20, 2015
    @larryhastings
    Copy link
    Contributor

    I'm not opposed to the patch in principle. I assume your goal is to make Python faster--do you have any data on how much faster?

    I don't support immediately changing all uses of Argument Clinic to generate their code into a separate file. I would want to see a consensus from the community first. Perhaps we should discuss it (again?) on python-dev?

    @serhiy-storchaka
    Copy link
    Member Author

    This is one step on long way. Second step will be to inline PyArg_Parse for
    some format codes ("i", "U", "y*", "O&", "O!"). Then we could try to expand
    PyArg_ParseTuple, at least for simple common cases. Then
    PyArg_ParseTupleAndKeywords. All this step will produce large diffs for
    generated code.

    @scoder
    Copy link
    Contributor

    scoder commented Feb 21, 2015

    Serhiy, I suggest you look at the code that Cython generates for its functions. It has been extensively profiled and optimised (years ago), so generating the same code for the argument clinic should yield the same performance.

    And while I don't have exact numbers at hand, avoiding the tuple packing for the call by passing it into a METH_O function can make a substantial difference. It also kills support for keyword arguments, though.

    @larryhastings
    Copy link
    Contributor

    Stefan: Serhiy's patch only affects functions taking a single positional-only parameter.

    @larryhastings
    Copy link
    Contributor

    lgtm

    @serhiy-storchaka
    Copy link
    Member Author

    Serhiy, I suggest you look at the code that Cython generates for its functions. It has been extensively profiled and optimised (years ago), so generating the same code for the argument clinic should yield the same performance.

    Thanks, I'll look on it.

    And while I don't have exact numbers at hand, avoiding the tuple packing for the call by passing it into a METH_O function can make a substantial difference.

    Good idea. Here are samples:

    $ ./python -m timeit "chr(0x20ac)"
    Unpatched: 1000000 loops, best of 3: 0.976 usec per loop
    Patched:   1000000 loops, best of 3: 0.752 usec per loop
    
    $ ./python -m timeit -s "from cmath import isnan; x = 1j" -- "isnan(x)"
    Unpatched: 1000000 loops, best of 3: 0.62 usec per loop
    Patched:   1000000 loops, best of 3: 0.386 usec per loop

    Of course for more complex functions the effect is smaller.

    @serhiy-storchaka
    Copy link
    Member Author

    After expanding PyArg_Parse for "i" and "D" codes above tests give following results:

    $ ./python -m timeit "chr(0x20ac)"
    1000000 loops, best of 3: 0.558 usec per loop
    
    $ ./python -m timeit -s "from cmath import isnan; x = 1j" -- "isnan(x)"
    1000000 loops, best of 3: 0.278 usec per loop

    About twice in comparison with initial variant!

    @serhiy-storchaka serhiy-storchaka self-assigned this Feb 22, 2015
    @pitrou
    Copy link
    Member

    pitrou commented Feb 22, 2015

    Let's make Argument Clinic a fierce optimizer!
    (+1 on this)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 3, 2015

    New changeset e10ad4d4d490 by Serhiy Storchaka in branch 'default':
    Issue bpo-23492: Argument Clinic now generates argument parsing code with
    https://hg.python.org/cpython/rev/e10ad4d4d490

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 4, 2015

    New changeset 973c9ec53bbb by Serhiy Storchaka in branch 'default':
    Fixed the array module broken in issue bpo-23492.
    https://hg.python.org/cpython/rev/973c9ec53bbb

    @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
    build The build process and cross-build topic-argument-clinic type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants