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

Optimize _PyFunction_FastCallDict() for **kwargs #73504

Closed
vstinner opened this issue Jan 19, 2017 · 8 comments
Closed

Optimize _PyFunction_FastCallDict() for **kwargs #73504

vstinner opened this issue Jan 19, 2017 · 8 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@vstinner
Copy link
Member

BPO 29318
Nosy @vstinner, @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 2017-02-01.16:08:45.272>
created_at = <Date 2017-01-19.11:33:01.385>
labels = ['interpreter-core', '3.7', 'performance']
title = 'Optimize _PyFunction_FastCallDict() for **kwargs'
updated_at = <Date 2017-02-24.18:30:40.852>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2017-02-24.18:30:40.852>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2017-02-01.16:08:45.272>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2017-01-19.11:33:01.385>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 29318
keywords = []
message_count = 8.0
messages = ['285773', '285777', '285782', '286642', '286643', '286654', '286661', '286666']
nosy_count = 4.0
nosy_names = ['vstinner', 'methane', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue29318'
versions = ['Python 3.7']

@vstinner
Copy link
Member Author

Example:
---

def func(x, y):
    print(x, y)

def proxy2(func, **kw):
    func(**kw)

def proxy1(func, **kw):
    proxy2(func, **kw)

The "proxy2(func, **kw)" call in proxy1() is currently inefficient: _PyFunction_FastCallDict() converts the dictionary into a C array [key1, value1, key2, value2, ...] and then _PyEval_EvalCodeWithName() rebuilds the dictionary from the C array.

Since "func(*args, **kw)" proxies are common in Python, especially to call the parent constructor when overriding __init__, I think that it would be interesting to optimize this code path.

I first expected that it was a regression of FASTCALL, but Python < 3.6 doesn't optimize this code neither.

@vstinner vstinner added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Jan 19, 2017
@methane
Copy link
Member

methane commented Jan 19, 2017

Since mutating kw dict shouldn't affect caller's dict, caller and callee can't share the dict.

One possible optimization is using PyDict_Copy() to copy the dict.
It can reduce number of hash() calls.

@serhiy-storchaka
Copy link
Member

It can reduce number of hash() calls.

Keys are strings (even interned strings), and hashes are cached.

In most cases kw is empty or very small. I doubt any optimization can have significant effect.

@python-dev
Copy link
Mannequin

python-dev mannequin commented Feb 1, 2017

New changeset fcba9c9d987b by Victor Stinner in branch 'default':
Document that _PyFunction_FastCallDict() must copy kwargs
https://hg.python.org/cpython/rev/fcba9c9d987b

@vstinner
Copy link
Member Author

vstinner commented Feb 1, 2017

INADA Naoki: "Since mutating kw dict shouldn't affect caller's dict, caller and callee can't share the dict."

Oh, I knew that, it's not the first time that I propose to implement this optimization and that I got this answer !? So I added a comment for myself, to remind me not to propose this optimization anymore :-D

I close this issue.

Serhiy Storchaka: "In most cases kw is empty or very small. I doubt any optimization can have significant effect."

I have no opinion on Naoki's optimization idea.

Naoki: Feel free to implement your idea to measure the speedup, but please open a new issue for that if you consider that it's worth it.

FYI Naoki and me started to list ideas to optimize CPython 3.7:
http://faster-cpython.readthedocs.io/cpython37.html

@vstinner vstinner closed this as completed Feb 1, 2017
@serhiy-storchaka
Copy link
Member

Changes by Roundup Robot <devnull@psf.upfronthosting.co.za>:

Why Roundup Robot spams with empty or almost empty messages?

@vstinner
Copy link
Member Author

vstinner commented Feb 1, 2017

Serhiy Storchaka: "Why Roundup Robot spams with empty or almost empty messages?"

It smells like the migration to GitHub started :-)

@python-dev
Copy link
Mannequin

python-dev mannequin commented Apr 9, 2022

New changeset 2c145e9eda756aa2001282d7c016e740dd00a2d7 by Victor Stinner in branch 'master':
Document that _PyFunction_FastCallDict() must copy kwargs
2c145e9

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

No branches or pull requests

3 participants