Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Oct 13, 2017

Remove the cached tuple which caused crashed, use
_PyObject_FastCall() instead.

Microbenchmark using:

./python -m perf timeit
    -s 'import collections;P=collections.namedtuple("P","x y");p=P(1, 2)'
    'p.x'

[ref] 80.4 ns +- 3.3 ns -> [fastcall] 103 ns +- 5 ns: 1.28x slower (+28%)

https://bugs.python.org/issue30156

Remove the cached tuple which caused crashed, use
_PyObject_FastCall() instead.

Microbenchmark using:

    ./python -m perf timeit
        -s 'import collections;P=collections.namedtuple("P","x y");p=P(1, 2)'
        'p.x'

[ref] 80.4 ns +- 3.3 ns -> [fastcall] 103 ns +- 5 ns: 1.28x slower (+28%)
@vstinner vstinner added the type-bug An unexpected behavior, bug, or error label Oct 13, 2017
@vstinner
Copy link
Member Author

What do you think of this bugfix @serhiy-storchaka? Is it an acceptable slowdown to avoid any crash in this code? I prefer to not play with the devil: reference counts and garbage collector.

@vstinner
Copy link
Member Author

Ping @serhiy-storchaka: would you be ok to remove the optimization to fix the bug, and prevent future other bugs in corner cases?

@serhiy-storchaka
Copy link
Member

There are ways of fixing a bug without hitting the performance.

@vstinner vstinner closed this Feb 1, 2018
@vstinner vstinner deleted the property_fastcall branch May 29, 2018 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting merge skip news type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants