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

Convert int.__round__ to Argument Clinic #85514

Closed
serhiy-storchaka opened this issue Jul 19, 2020 · 4 comments
Closed

Convert int.__round__ to Argument Clinic #85514

serhiy-storchaka opened this issue Jul 19, 2020 · 4 comments
Labels
3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage topic-argument-clinic

Comments

@serhiy-storchaka
Copy link
Member

BPO 41342
Nosy @rhettinger, @mdickinson, @larryhastings, @serhiy-storchaka
PRs
  • bpo-41342: Convert int.__round__ to Argument Clinic #21549
  • 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 2020-07-20.12:58:03.715>
    created_at = <Date 2020-07-19.12:38:29.250>
    labels = ['interpreter-core', 'expert-argument-clinic', '3.10', 'performance']
    title = 'Convert int.__round__ to Argument Clinic'
    updated_at = <Date 2020-07-20.12:58:03.715>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2020-07-20.12:58:03.715>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-07-20.12:58:03.715>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core', 'Argument Clinic']
    creation = <Date 2020-07-19.12:38:29.250>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 41342
    keywords = ['patch']
    message_count = 4.0
    messages = ['373963', '373977', '373982', '374006']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'mark.dickinson', 'larry', 'serhiy.storchaka']
    pr_nums = ['21549']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue41342'
    versions = ['Python 3.10']

    @serhiy-storchaka
    Copy link
    Member Author

    int.__round__ was not converted to Argument Clinic because it is not impossible to express a correct signature for it in Python. But now we can at least make Argument Clinic not producing incorrect signature. And converting to Argument Clinic has a performance benefit.

    $ ./python -m pyperf timeit -q --compare-to=../cpython-baseline/python "round(12345)"
    Mean +- std dev: [/home/serhiy/py/cpython-baseline/python] 123 ns +- 6 ns -> [/home/serhiy/py/cpython-release/python] 94.4 ns +- 2.4 ns: 1.31x faster (-23%)
    
    $ ./python -m pyperf timeit -q --compare-to=../cpython-baseline/python "round(12345, 0)"
    Mean +- std dev: [/home/serhiy/py/cpython-baseline/python] 159 ns +- 4 ns -> [/home/serhiy/py/cpython-release/python] 98.6 ns +- 2.4 ns: 1.61x faster (-38%)
    
    $ ./python -m pyperf timeit -q --compare-to=../cpython-baseline/python "round(12345, -2)"
    Mean +- std dev: [/home/serhiy/py/cpython-baseline/python] 585 ns +- 9 ns -> [/home/serhiy/py/cpython-release/python] 534 ns +- 14 ns: 1.09x faster (-9%)

    @serhiy-storchaka serhiy-storchaka added 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-argument-clinic performance Performance or resource usage labels Jul 19, 2020
    @rhettinger
    Copy link
    Contributor

    I don't have an opinion on the PR but want to point-out the Argument Clinic itself doesn't provide a performance benefit. Anything that it does can also be done directly by the code itself, including vectorcall logic. You should be able to optimize the function calls without using the Argument Clinic.

    That said, it would be great if someone were to work on building-out AC to support more interesting argument patterns like those in round(). Ideally, AC would provide complete, correct support right out of the box.

    @serhiy-storchaka
    Copy link
    Member Author

    Yes, you can do this without Argument Clinic, but Argument Clinic allows to hide the use of unstable private API and cumbersome code under macros and in generated files.

    That said, it would be great if someone were to work on building-out AC to support more interesting argument patterns like those in round().

    The problem is not only in Argument Clinic (and the Argument Clinic part is mainly solved). The problem is that currently it is not possible to express the signature of int.__round__() (and dict.get(), and getattr()) in Python syntax, and the inspect module does not support any non-Python syntax for this either. Once we invent the way to express it, supporting it in Argument Clinic will be merely technical problem.

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 5a2bac7 by Serhiy Storchaka in branch 'master':
    bpo-41342: Convert int.__round__ to Argument Clinic (GH-21549)
    5a2bac7

    @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.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage topic-argument-clinic
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants