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

Use PEP 590 vectorcall to speed up GenericAlias. #84549

Closed
corona10 opened this issue Apr 23, 2020 · 8 comments
Closed

Use PEP 590 vectorcall to speed up GenericAlias. #84549

corona10 opened this issue Apr 23, 2020 · 8 comments
Assignees
Labels
3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@corona10
Copy link
Member

BPO 40369
Nosy @gvanrossum, @vstinner, @serhiy-storchaka, @corona10
PRs
  • bpo-40369: Use PEP 590 vectorcall to speed up GenericAlias #19677
  • Files
  • bench_generic_alias.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 = 'https://github.com/corona10'
    closed_at = <Date 2020-04-23.16:27:29.480>
    created_at = <Date 2020-04-23.02:48:54.393>
    labels = ['interpreter-core', '3.9', 'performance']
    title = 'Use PEP 590 vectorcall to speed up GenericAlias.'
    updated_at = <Date 2020-04-23.16:27:29.480>
    user = 'https://github.com/corona10'

    bugs.python.org fields:

    activity = <Date 2020-04-23.16:27:29.480>
    actor = 'corona10'
    assignee = 'corona10'
    closed = True
    closed_date = <Date 2020-04-23.16:27:29.480>
    closer = 'corona10'
    components = ['Interpreter Core']
    creation = <Date 2020-04-23.02:48:54.393>
    creator = 'corona10'
    dependencies = []
    files = ['49087']
    hgrepos = []
    issue_num = 40369
    keywords = ['patch']
    message_count = 8.0
    messages = ['367074', '367103', '367104', '367106', '367107', '367108', '367109', '367128']
    nosy_count = 4.0
    nosy_names = ['gvanrossum', 'vstinner', 'serhiy.storchaka', 'corona10']
    pr_nums = ['19677']
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue40369'
    versions = ['Python 3.9']

    @corona10
    Copy link
    Member Author

    Since PEP-560 was approved,
    The following syntax has become available.

    from queue import Queue
    v = Queue[int]

    It's a very shiny feature.
    Given the direction of programming language, it's probably a very useful feature that users will use very often in the near future.

    When I looked at Object/geneticiasobject.c, the current implementation is a very good condition for using the vectorcall proposed by PEP-590 .

    The benchmarks are as follows.
    So I suggest using vectorcall in GenericAlias.

    if the suggestion is accepted, I 'd like to summit the patch :)

    +--------------------+----------------------+-----------------------------+
    | Benchmark | master-generic-alias | proposed-generic-alias |
    +====================+======================+=============================+
    | bench GenericAlias | 143 ns | 111 ns: 1.29x faster (-22%) |
    +--------------------+----------------------+-----------------------------+

    @corona10 corona10 added 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Apr 23, 2020
    @corona10 corona10 self-assigned this Apr 23, 2020
    @corona10 corona10 added the performance Performance or resource usage label Apr 23, 2020
    @corona10 corona10 self-assigned this Apr 23, 2020
    @corona10 corona10 added the performance Performance or resource usage label Apr 23, 2020
    @vstinner
    Copy link
    Member

    if the suggestion is accepted, I 'd like to summit the patch :)

    Can you propose a PR, so I can have a look at the implementation?

    @corona10
    Copy link
    Member Author

    Can you propose a PR, so I can have a look at the implementation?

    Got it :), I will upload it soon.

    @serhiy-storchaka
    Copy link
    Member

    I have doubts that GenericAlias instances are created in mass in tight loops. Do you have any realistic example which would benefit from speeding up creation of GenericAlias instances?

    @corona10
    Copy link
    Member Author

    Do you have any realistic example which would benefit from speeding up the creation of GenericAlias instances?

    Hmm I did not find loop tightly calling case, but
    for example, this kinds of usage are very often.

    def create_q(l: list[int]) -> Queue[int]:
        q = Queue[int]()
        for e in l:
            q.put(e)
        return q
    
    a = create_q([1,2,3,4,5])

    In this code, GenericAlias is called 2 times for function declare and function call.

    It's true that GenericAlias ​​has a small portion in this scenario,
    but wouldn't it be worth it if we could optimize itself to a few lines of code?

    @serhiy-storchaka
    Copy link
    Member

    And how much faster has your example become?

    @corona10
    Copy link
    Member Author

    And how much faster has your example become?

    For function declaration, 5% enhancement,
    the function call does not show difference because the GenericAlias is not big portion on calling.

    +--------------------+------------------------+----------------------------+
    | Benchmark | master-generic-alias-1 | bpo-40369-generic-alias-1 |
    +====================+========================+============================+
    | bench GenericAlias | 420 ns | 399 ns: 1.05x faster (-5%) |
    +--------------------+------------------------+----------------------------+

    @corona10
    Copy link
    Member Author

    Just for the record,

    #19677 (comment)

    guido left an opinion that the caching approach might be more proper.
    So I 'd like to suggest open a new issue for caching. :)

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

    No branches or pull requests

    3 participants