Add PyObject_CallOneArg()

@vstinner

BPO	37483
Nosy	@vstinner, @methane, @skrah, @jdemeyer, @corona10, @miss-islington
PRs	bpo-37483: add _PyObject_CallOneArg() function #14558 bpo-37493: use _PyObject_CallNoArg in more places #14575 bpo-37483: fix refleak #14599 bpo-37483: fix reference leak in _PyCodec_Lookup #14600 bpo-37483: Add PyObject_CallOneArg() in the What's New in Python 3.9 #23062 [3.9] bpo-37483: Add PyObject_CallOneArg() in the What's New in Python 3.9 (GH-23062) #23074

^{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-11-01.13:28:37.261>
created_at = <Date 2019-07-02.12:14:43.792>
labels = ['interpreter-core', '3.9', 'performance']
title = 'Add PyObject_CallOneArg()'
updated_at = <Date 2020-11-01.13:28:37.261>
user = 'https://github.com/jdemeyer'

bugs.python.org fields:

activity = <Date 2020-11-01.13:28:37.261>
actor = 'corona10'
assignee = 'none'
closed = True
closed_date = <Date 2020-11-01.13:28:37.261>
closer = 'corona10'
components = ['Interpreter Core']
creation = <Date 2019-07-02.12:14:43.792>
creator = 'jdemeyer'
dependencies = []
files = []
hgrepos = []
issue_num = 37483
keywords = ['patch']
message_count = 32.0
messages = ['347138', '347172', '347181', '347196', '347197', '347198', '347200', '347202', '347203', '347205', '347206', '347208', '347209', '347210', '347211', '347213', '347214', '347216', '347217', '347220', '347222', '347224', '347231', '347267', '347318', '347330', '347332', '347333', '347335', '370685', '380132', '380133']
nosy_count = 6.0
nosy_names = ['vstinner', 'methane', 'skrah', 'jdemeyer', 'corona10', 'miss-islington']
pr_nums = ['14558', '14575', '14599', '14600', '23062', '23074']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue37483'
versions = ['Python 3.9']

As discussed in https://mail.python.org/archives/list/capi-sig@python.org/thread/X6HJOEX6RRFLNKAQSQR6HLD7K4QZ4OTZ/ it would be convenient to have a function PyObject_CallOneArg() for making calls with exactly 1 positional argument and no keyword arguments. Such calls are very common. This would be analogous to PyObject_CallNoArgs().

What do you think about macro like this?

  #define _PyObject_CALL_WITH_ARGS(func, ...) \
      _PyObject_Vectorcall(func, (PyObject*[]){NULL, __VA_ARGS__} + 1, \
              sizeof((PyObject*[]){__VA_ARGS__})/sizeof(PyObject*) | PY_VECTORCALL_ARGUMENTS_OFFSET, \
              NULL)

Pros: it can be used for 2 or 3 arguments too.
Cons: readability...

Variadic macros are not part of C89, so that would require changing PEP-7.

Cons: readability...

It's a single relatively short macro. I'm not worried about that. I'm more about compiler support for such macros. I'll make a PR with this idea and see what CI says.

Variadic macros are not part of C89, so that would require changing PEP-7.

PEP-7 uses C99 since Python 3.6:
https://www.python.org/dev/peps/pep-0007/#c-dialect

What do you think about macro like this?

Macros cannot be used in all C extensions. For the PEP-587, I was asked to replace macros with functions.

I would prefer a PyObject_CallOneArg() function than yet another ugly and dangerous macro. Sometimes, a regular function is better for stack consumption: it doesn't increase the stack consumption in the call site, stack memory is "released" once PyObject_CallOneArg() exit. In the past, I had to disable inlining in some functions to reduce the stack consumption. (I'm not sure if this optimization is still around.)

AFAIK, gcc, clang, and MSVC support it.

Another cons is, general pitfall of macro:

_PyObject_CALL_WITH_ARGS(func, PyDict_GetItem(d, key)); // PyDict_GetItem(d, key) is called twice.

If two or more arguments are not common, I prefer _PyObject_CallOneArg to macro.

_PyObject_CALL_WITH_ARGS(func, PyDict_GetItem(d, key)); // PyDict_GetItem(d, key) is called twice.

That's pretty bad and in my opinion a good reason to reject this idea.

If two or more arguments are not common, I prefer _PyObject_CallOneArg to macro.

I posted some counts at https://mail.python.org/archives/list/capi-sig@python.org/message/3I76R27YMFSKB5JQIM3E4NA3BECVTZHP/

_PyObject_CALL_WITH_ARGS(func, PyDict_GetItem(d, key)); // PyDict_GetItem(d, key) is called twice.

Actually, it's not a problem: sizeof() is special, it only looks at the type of its argument, it doesn't execute the code.

PEP-7 uses C99 since Python 3.6: https://www.python.org/dev/peps/pep-0007/#c-dialect

That's not what the PEP says: "Python versions greater than or equal to 3.6 use C89 with several select C99 features"

"several select C99 features" is not the same of using C99.

Plus, I'm also worried about C++. We also support compiling C++ extensions with the same header files, so it must be C++ compatible also.

Macros cannot be used in all C extensions. For the PEP-587, I was asked to replace macros with functions.

This is just an helper inline function / macro to ease calling _PyObject_Vectorcall.
Extensions can port this helper inline function / macro to ease calling _PyObject_Vectorcall.

I don't want to make this non-inline function.

The motivation for this PR is "it would be convenient to have this function".

This is probably true, but generally I would not change 47 files at once.

Most of the locations are probably not performance sensitive.

This change doesn't make caller code complicated. It makes caller little simpler.

Choosing performance sensitive code is much hard than replace all occurrences.
So I'm OK to change all callers except code owner opposed at once.

Victor, what's your opinion on adding PyObject_CallOneArg() to the limited API?

Test of stack usage:

from _testcapi import stack_pointer
class D(dict):
    def __missing__(self, key):
        sp = stack_pointer()
        print(f"stack usage = {TOP - sp}")
        return None
d = D()
TOP = stack_pointer()
d[0]

**before**: stack usage = 1296
**after**: stack usage = 976

New changeset 196a530 by Inada Naoki (Jeroen Demeyer) in branch 'master':
bpo-37483: add _PyObject_CallOneArg() function (bpo-14558)
196a530

The following commit introduced a reference leak:

commit 196a530
Author: Jeroen Demeyer <J.Demeyer@UGent.be>
Date: Thu Jul 4 12:31:34 2019 +0200

bpo-37483: add \_PyObject_CallOneArg() function (bpo-14558)

Example:

https://buildbot.python.org/all/#/builders/80/builds/642

test_xml_etree leaked [1, 1, 1] references, sum=3
test_threading leaked [6, 6, 6] references, sum=18
test_atexit leaked [6, 6, 6] references, sum=18
test_compile leaked [1, 1, 1] references, sum=3
test_zipfile leaked [1, 1, 1] references, sum=3
test_tokenize leaked [3, 3, 3] references, sum=9
test_pydoc leaked [1, 1, 1] references, sum=3
test_codecs leaked [43, 43, 43] references, sum=129
test_codecs leaked [11, 11, 11] memory blocks, sum=33
Re-running failed tests in verbose mode
test__xxsubinterpreters leaked [136, 136, 136] references, sum=408
test_getargs2 leaked [4, 4, 4] references, sum=12
test_source_encoding leaked [3, 3, 3] references, sum=9
test_capi leaked [4, 4, 4] references, sum=12
test_email leaked [17, 17, 17] references, sum=51
test_xml_etree leaked [1, 1, 1] references, sum=3
test_threading leaked [6, 6, 6] references, sum=18
test_atexit leaked [6, 6, 6] references, sum=18
test_compile leaked [1, 1, 1] references, sum=3
test_zipfile leaked [1, 1, 1] references, sum=3
test_tokenize leaked [3, 3, 3] references, sum=9
test_pydoc leaked [1, 1, 1] references, sum=3
test_codecs leaked [43, 43, 43] references, sum=129
test_codecs leaked [11, 11, 11] memory blocks, sum=33
(...)

INADA-san, Jeroen: hum, you both proposed a similar fix :-) One fix is enough. See my review on PR 14600.

Jeroen: hum, you both proposed a similar fix :-)

It seems that I lost the race ;-)

But seriously: if we both independently came up with the same solution, that's a good sign that the solution is correct.

New changeset 6e43d07 by Inada Naoki (Jeroen Demeyer) in branch 'master':
bpo-37483: fix reference leak in _PyCodec_Lookup (GH-14600)
6e43d07

Thanks for the quick fix ;-)

Oh, PyObject_CallOneArg() is missing in the What's New in Python 3.9:
https://docs.python.org/dev/whatsnew/3.9.html#id1

Can someone please propose a PR to add it? I reopen the issue.

New changeset 7feb54a by Dong-hee Na in branch 'master':
bpo-37483: Add PyObject_CallOneArg() in the What's New in Python 3.9 (GH-23062)
7feb54a

New changeset 0312efc by Miss Skeleton (bot) in branch '3.9':
bpo-37483: Add PyObject_CallOneArg() in the What's New in Python 3.9 (GH-23062)
0312efc

Navigation Menu

Repository navigation

Add PyObject_CallOneArg() #81664

9 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Add PyObject_CallOneArg() #81664

Description

Activity

jdemeyer commented on Jul 2, 2019

methane commented on Jul 3, 2019

jdemeyer commented on Jul 3, 2019

jdemeyer commented on Jul 3, 2019

vstinner commented on Jul 3, 2019

methane commented on Jul 3, 2019

jdemeyer commented on Jul 3, 2019

jdemeyer commented on Jul 3, 2019

jdemeyer commented on Jul 3, 2019

methane commented on Jul 3, 2019

skrah commented on Jul 3, 2019

methane commented on Jul 3, 2019

9 remaining items

jdemeyer commented on Jul 3, 2019

jdemeyer commented on Jul 3, 2019

methane commented on Jul 4, 2019

vstinner commented on Jul 5, 2019

vstinner commented on Jul 5, 2019

jdemeyer commented on Jul 5, 2019

methane commented on Jul 5, 2019

vstinner commented on Jul 5, 2019

vstinner commented on Jun 3, 2020

corona10 commented on Nov 1, 2020

miss-islington commented on Nov 1, 2020

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions