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: inline PyArg_UnpackTuple and PyArg_ParseStack(AndKeyword)? #73605

Closed
vstinner opened this issue Feb 2, 2017 · 2 comments
Closed
Labels
3.7 (EOL) end of life performance Performance or resource usage topic-argument-clinic

Comments

@vstinner
Copy link
Member

vstinner commented Feb 2, 2017

BPO 29419
Nosy @vstinner, @larryhastings, @methane, @serhiy-storchaka

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 2018-09-19.23:20:17.169>
created_at = <Date 2017-02-02.12:50:49.351>
labels = ['3.7', 'expert-argument-clinic', 'performance']
title = 'Argument Clinic: inline PyArg_UnpackTuple and PyArg_ParseStack(AndKeyword)?'
updated_at = <Date 2018-09-19.23:20:17.168>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2018-09-19.23:20:17.168>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-09-19.23:20:17.169>
closer = 'vstinner'
components = ['Argument Clinic']
creation = <Date 2017-02-02.12:50:49.351>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 29419
keywords = []
message_count = 2.0
messages = ['286778', '286779']
nosy_count = 4.0
nosy_names = ['vstinner', 'larry', 'methane', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue29419'
versions = ['Python 3.7']

@vstinner
Copy link
Member Author

vstinner commented Feb 2, 2017

Argument Clinic calls one the following functions depending on parameters:

  • PyArg_UnpackTuple(), _PyArg_UnpackStack()
  • PyArg_ParseTuple(), _PyArg_ParseStack()
  • PyArg_ParseTupleAndKeywords(), _PyArg_ParseStackAndKeywords()
  • etc.

Would it make sense to emit C code instead of calling complex and slow PyArg_ParseXXX() functions? It would emit the most efficient C code to parse arguments.

I don't recall where this idea comes from. Maybe Larry Hastings told me once that he wants to implement this idea :-) I'm sure that Larry has a big plan but lacks time to implement all of his cool ideas.

Using profiled guided optimization (PGO), the compiler should be able to easily detect that error cases are unlikely and mark these code paths as unlikely.

We should probably experiment an implementation to be able to measure the speedup, to be able to say if the idea is worth it or not, in term of performance, since the motivation here is clearly performance.

We can begin with format strings only made of "O" format. Most simple example with divmod(), replace:

    if (!_PyArg_UnpackStack(args, nargs, "divmod", 2, 2, &x, &y)) { return NULL; }
with something like:
    if (nargs != 2) { _PyArg_ErrNumArgs(nargs, 2, 2, "divmod"); return NULL; }
    x = args[0];
    y = args[1];

The next question is if we should go further with more complex formats. Example with the format() function, replace:

    if (!_PyArg_ParseStack(args, nargs, "O|U:format", &value, &format_spec)) { ... }

with:

    if (nargs < 1 || nargs > 2) { _PyArg_ErrNumArgs(nargs, 1, 2, "format"); return NULL; }
    value = args[0];
    if (nargs == 2) {
        format_spec = args[1];
        if (!PyUnicode_Check(format_spec)) { .. raise an exception ...; return NULL; }
        /* getargs.c calls PyUnicode_READY(), we should also do it here */
        if (PyUnicode_READY(format_spec) == -1) { return NULL; }
    }
    else {
        format_spec = NULL;
    }

@vstinner vstinner added 3.7 (EOL) end of life topic-argument-clinic performance Performance or resource usage labels Feb 2, 2017
@serhiy-storchaka
Copy link
Member

See bpo-23867 for the first step (unfinished). After that I planned to inline parsing code for PyArg_ParseTuple() with multiple arguments. Then split PyArg_ParseTupleAndKeywords() on two parts: first unpack keywords to linear sparse array and then handle it as positional arguments.

@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 performance Performance or resource usage topic-argument-clinic
Projects
None yet
Development

No branches or pull requests

2 participants