8000 Optimize builtin functions min() and max() · Issue #90350 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

Optimize builtin functions min() and max() #90350

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.

Sign up for GitHub

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

Jump to bottom
Closed
colorfulappl mannequin opened this issue Dec 29, 2021 · 5 comments
Closed

Optimize builtin functions min() and max() #90350

colorfulappl mannequin opened this issue Dec 29, 2021 · 5 comments
Labels
3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage topic-argument-clinic

Comments

@colorfulappl
Copy link
Mannequin
colorfulappl mannequin commented Dec 29, 2021
BPO 46192
Nosy @sweeneyde, @erlend-aasland, @colorfulappl
PRs
  • gh-90350: Optimize builtin functions min() and max() #30286
    		•
    Files
  • bench-minmax.py
    		•

    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 = None
created_at = <Date 2021-12-29.10:04:55.181>
labels = ['interpreter-core', '3.11', 'performance']
title = 'Optimize builtin functions min() and max()'
updated_at = <Date 2021-12-31.14:22:47.367>
user = 'https://github.com/colorfulappl'

    bugs.python.org fields:

    activity = <Date 2021-12-31.14:22:47.367>
actor = 'Dennis Sweeney'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2021-12-29.10:04:55.181>
creator = 'colorfulappl'
dependencies = []
files = ['50528']
hgrepos = []
issue_num = 46192
keywords = ['patch']
message_count = 3.0
messages = ['409299', '409367', '409421']
nosy_count = 3.0
nosy_names = ['Dennis Sweeney', 'erlendaasland', 'colorfulappl']
pr_nums = ['30286']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue46192'
versions = ['Python 3.11']
    @colorfulappl
    Copy link
    Mannequin Author
    colorfulappl mannequin commented Dec 29, 2021

    faster-cpython/ideas#199

    @colorfulappl colorfulappl mannequin added 3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Dec 29, 2021
    @erlend-aasland
    Copy link
    Contributor

    Repeating my comment on #74472: If we are touching min() and max(), it would make sense, IMO, to port them to Argument Clinic. FTR, Argument Clinic got *args support in #62809 / bpo-20291.

    @colorfulappl made a "competing" branch using AC on his local fork1. However, that branch contained a bug with the key function; I made an amended version2 for benchmarking. Here's some micro-benchmarks from optimised builds on macOS 12.1 w/Clang 13:

    +---------------------------------------------------------+---------+-----------------------+----------------------+
| Benchmark                                               | main    | python/cpython#74472              | python/cpython#74472-ac          |
+=========================================================+=========+=======================+======================+
| max(a, b)                                               | 193 ns  | 74.1 ns: 2.60x faster | 179 ns: 1.08x faster |
+---------------------------------------------------------+---------+-----------------------+----------------------+
| max(a, b, c, d, e)                                      | 273 ns  | 126 ns: 2.17x faster  | 260 ns: 1.05x faster |
+---------------------------------------------------------+---------+-----------------------+----------------------+
| max([a, b])                                             | 267 ns  | 185 ns: 1.44x faster  | 239 ns: 1.12x faster |
+---------------------------------------------------------+---------+-----------------------+----------------------+
| max([a, b, c, d, e])                                    | 345 ns  | 259 ns: 1.33x faster  | 312 ns: 1.10x faster |
+---------------------------------------------------------+---------+-----------------------+----------------------+
| max((a,), (b,), key=lambda x: x[0])                     | 707 ns  | 444 ns: 1.59x faster  | 513 ns: 1.38x faster |
+---------------------------------------------------------+---------+-----------------------+----------------------+
| max((a,), (b,), (c,), (d,), (e,), key=lambda x: x[0])   | 1.12 us | 831 ns: 1.35x faster  | 930 ns: 1.20x faster |
+---------------------------------------------------------+---------+-----------------------+----------------------+
| max([(a,), (b,)], key=lambda x: x[0])                   | 786 ns  | 561 ns: 1.40x faster  | 570 ns: 1.38x faster |
+---------------------------------------------------------+---------+-----------------------+----------------------+
| max([(a,), (b,), (c,), (d,), (e,)], key=lambda x: x[0]) | 1.19 us | 981 ns: 1.22x faster  | 981 ns: 1.22x faster |
+---------------------------------------------------------+---------+-----------------------+----------------------+
| max([], default=-1)                                     | 484 ns  | 177 ns: 2.73x faster  | 188 ns: 2.57x faster |
+---------------------------------------------------------+---------+-----------------------+----------------------+
| Geometric mean                                          | (ref)   | 1.68x faster          | 1.29x faster         |
+---------------------------------------------------------+---------+-----------------------+----------------------+

    Footnotes

    1. https://github.com/colorfulappl/cpython/commit/29b9559de31ae19e8d127d7a3063494b2d9791b0 ↩

    2. https://github.com/erlend-aasland/cpython/tree/gh-30286-ac ↩

    @sweeneyde
    Copy link
    Member

    Kind of like what's mentioned at faster-cpython/ideas#131 , it may be interesting to explore implementing min()/max() in Pure python, considering recent specializations to COMPARE_OP, and potential future specializations of FOR_ITER.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel iritkatriel added 3.12 only security fixes and removed 3.11 only security fixes labels Sep 7, 2022
    @bedevere-bot bedevere-bot mentioned this issue Feb 26, 2023
    Merged
    @arhadthedev
    Copy link
    Member

    @erlend-aasland Would you mind to publish https://github.com/erlend-aasland/cpython/tree/gh-30286-ac you mentioned as a PR, please? It will be a great opportunity to close gh-30286 and resolve this issue for good.

    serhiy-storchaka pushed a commit that referenced this issue Dec 11, 2023
    @colorfulappl
    gh-90350: Optimize builtin functions min() and max() (GH-30286)
    0066ab5 
    Builtin functions min() and max() now use METH_FASTCALL
    @serhiy-storchaka
    Copy link
    Member

    Thank you for your contribution @colorfulappl.

    @serhiy-storchaka serhiy-storchaka added 3.13 bugs and security fixes and removed 3.12 only security fixes labels Dec 11, 2023
    facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this issue Jan 16, 2024
    @itamaro @facebook-github-bot
    Backport gh-90350: Optimize builtin functions min() and max() (GH-30286)
    8476e31 
    Summary:
Builtin functions min() and max() now use METH_FASTCALL

upstream issue: python/cpython#90350
upstream PR: python/cpython#30286
upstream commit: python/cpython@0066ab5

Reviewed By: carljm

Differential Revision: D52650071

fbshipit-source-id: 93971e865ab9515efc9771c58582f63d15e0342d
    aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
    @colorfulappl @aisk
    pythongh-90350: Optimize builtin functions min() and max() (pythonGH-…
    b69fd78 
    …30286)

Builtin functions min() and max() now use METH_FASTCALL
    Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
    @colorfulappl @Glyphack
    pythongh-90350: Optimize builtin functions min() and max() (pythonGH-…
    c292b70 
    …30286)

Builtin functions min() and max() now use METH_FASTCALL
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage topic-argument-clinic
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @iritkatriel @serhiy-storchaka @arhadthedev @erlend-aasland @sweeneyde
    0