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

__slots__ make attribute setting 10x slower #46369

Closed
jyasskin mannequin opened this issue Feb 14, 2008 · 10 comments
Closed

__slots__ make attribute setting 10x slower #46369

jyasskin mannequin opened this issue Feb 14, 2008 · 10 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@jyasskin
Copy link
Mannequin

jyasskin mannequin commented Feb 14, 2008

BPO 2115
Nosy @theller, @facundobatista, @amauryfa, @tiran

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 2008-02-15.21:30:40.727>
created_at = <Date 2008-02-14.18:01:36.485>
labels = ['interpreter-core', 'performance']
title = '__slots__ make attribute setting 10x slower'
updated_at = <Date 2008-02-15.21:30:40.599>
user = 'https://bugs.python.org/jyasskin'

bugs.python.org fields:

activity = <Date 2008-02-15.21:30:40.599>
actor = 'amaury.forgeotdarc'
assignee = 'none'
closed = True
closed_date = <Date 2008-02-15.21:30:40.727>
closer = 'amaury.forgeotdarc'
components = ['Interpreter Core']
creation = <Date 2008-02-14.18:01:36.485>
creator = 'jyasskin'
dependencies = []
files = []
hgrepos = []
issue_num = 2115
keywords = []
message_count = 10.0
messages = ['62408', '62409', '62410', '62411', '62412', '62414', '62415', '62417', '62434', '62441']
nosy_count = 5.0
nosy_names = ['theller', 'facundobatista', 'amaury.forgeotdarc', 'christian.heimes', 'jyasskin']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue2115'
versions = ['Python 2.6']

@jyasskin
Copy link
Mannequin Author

jyasskin mannequin commented Feb 14, 2008

(On a MacBook Pro 2.33 GHz)

$ ./python.exe -m timeit -s 'class Foo(object): pass' -s 'f = Foo()'
'f.num = 3'
10000000 loops, best of 3: 0.13 usec per loop
$ ./python.exe -m timeit -s 'class Foo(object): __slots__ = ["num"]' -s
'f = Foo()' 'f.num = 3'
1000000 loops, best of 3: 1.24 usec per loop

Attribute reading isn't affected:
$ ./python.exe -m timeit -s 'class Foo(object): pass' -s 'f = Foo();
f.num = 3' 'g = f.num'
10000000 loops, best of 3: 0.107 usec per loop
$ ./python.exe -m timeit -s 'class Foo(object): __slots__ = ["num"]' -s
'f = Foo(); f.num = 3' 'g = f.num'
10000000 loops, best of 3: 0.101 usec per loop

@jyasskin jyasskin mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Feb 14, 2008
@amauryfa
Copy link
Member

This slowdown is due to the new implementation of isinstance in python2.6.

If I comment most of the code of PyObject_IsInstance in
Objects/abstract.c (remove all __instancecheck__ stuff; leave only the
last line), timings are much better, and more similar to 2.5 performance.

@theller
Copy link

theller commented Feb 14, 2008

I think this is a serious problem (and thanks, amaury, for finding the
spot). comtypes, like ctypes, uses quite a bit of isinstance calls.

It is the reason that the comtypes unit tests run between 8% and 25%
slower with trunk than with python 2.5.1. If I comment out the code
that you mentioned, python trunk is slightly faster than 2.5.1. I
suggest to raise the severity.

@tiran
Copy link
Member

tiran commented Feb 14, 2008

I agree, Thomas

@theller
Copy link

theller commented Feb 14, 2008

PyObject_IsSubclass() has the same problem.

BTW; calling PyObject_GetAttr() with interned strings for
"__instancecheck__" and "__subclasscheck__" brings not enough speedup.

@amauryfa
Copy link
Member

Something interesting:

  • attribute setting uses PyObject_IsInstance, which is slower since the
    introduction of ABCs.
  • attribute reading uses PyObject_TypeCheck, which only searches the
    __mro__.

Does this means that many calls to IsInstance could be replaced by
TypeCheck? Of course this makes sense only for new-style classes, but it
is the case for all built-in objects.

@amauryfa
Copy link
Member

With this trivial patch, all tests still pass:

Index: Objects/descrobject.c
===================================================================

--- Objects/descrobject.c       (revision 60754)
+++ Objects/descrobject.c       (working copy)
@@ -166,7 +166,7 @@
               int *pres)
 {
        assert(obj != NULL);
-       if (!PyObject_IsInstance(obj, (PyObject *)(descr->d_type))) {
+       if (!PyObject_TypeCheck(obj, descr->d_type)) {
                PyErr_Format(PyExc_TypeError,
                             "descriptor '%.200s' for '%.100s' objects "
                             "doesn't apply to '%.100s' object",

@tiran
Copy link
Member

tiran commented Feb 14, 2008

Thomas Heller wrote:

BTW; calling PyObject_GetAttr() with interned strings for
"__instancecheck__" and "__subclasscheck__" brings not enough speedup.

I've implemented the interning as static PyObject* in r60822. It should
give a small speedup.

@amauryfa
Copy link
Member

__instancecheck__ is not only slower, it can also cause crashes:

import abc
class MyABC:
    __metaclass__ = abc.ABCMeta
    __slots__ = ["a"]

class Unrelated:
    pass
MyABC.register(Unrelated)

u=Unrelated()
assert isinstance(u, MyABC)

MyABC.a.__set__(u, 3) # Boom

The patch I proposed above correctly raises the error:
TypeError: descriptor 'a' for 'MyABC' objects doesn't apply to
'Unrelated' object

@amauryfa
Copy link
Member

I think I fixed the issue.
"svn annotate" showed that the exact same optimization was applied on
__slots__ read access, five years ago: r28297.

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

No branches or pull requests

3 participants