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

Use super(cls, self) instead of super() or hard-coded class? #1061

Closed
kohr-h opened this issue Jul 5, 2017 · 2 comments
Closed

Use super(cls, self) instead of super() or hard-coded class? #1061

kohr-h opened this issue Jul 5, 2017 · 2 comments

Comments

@kohr-h
Copy link
Member

kohr-h commented Jul 5, 2017

So far we always use e.g. super().__init__(...) in subclasses and get Python 2 compatibility by importing a super backport from the future library. That is, however, slow, and thus a bunch of speed-critical classes already replace this by hard-coded ParentClass.__init__(self, ...) calls.

This is not very pretty and particularly unfriendly towards downstream subclassers. Thus, an alternative could be to use the built-in super in both Python 2 and 3 and stick to the (required in Py2, redundant but valid in Py3) syntax super(cls, self).__init__(...).

@adler-j do you have any good test lying around to check the speed of this compared to super from future and hard-coded parents?

@adler-j
Copy link
Member

adler-j commented Jul 5, 2017

I'm fine with any improvement to usability as long as we don't significantly regress performance.

As to tests, I don't, but when i Optimized this I simply did some profiling on the test suite and/or some of the examples in the examples suite.

Lately I've had the best experiance using yappi together with KCacheGrind.

@kohr-h
Copy link
Member Author

kohr-h commented Jul 5, 2017

Here are some very quick timing results.

I test the following inheritance chain (not totally trivial, needs to be traversed to the top):

class A(object):
    def __init__(self, a):
        self.a = a

class B(A):
    pass

class C(object):
    pass

class D1(C, A):
    def __init__(self, a, b):
        super().__init__(a)  # Invalid in Python 2
        self.b = b

class D2(C, A):
    def __init__(self, a, b):
        super(D2, self).__init__(a)  # Works with Python 2 and 3
        self.b = b
        
class D3(C, A):
    def __init__(self, a, b):
        A.__init__(self, a)  # Hard-coded, no MRO involved
        self.b = b

Running %timeit d = D1(1, 2) and equally for D2 and D3 give these results:

Python version D1 (super magic) D2 (super, explicit args) D3 (hard-coded)
2.7, built-in super 599 ns 555 ns
2.7,super from future 7.18 µs 729 ns 570 ns
3.6 650 ns 707 ns 479 ns

This seems to summarize quite well the experience from the past, namely that using the backported super in Python 2 really gives a significant performance hit when many objects need to be created. The remaining options are more or less on par. Skipping the MRO entirely is, of course, fastest, but not really by much. And the difference super with and without args is probably just storage of arguments and dictionary lookups that come on top for the former.

I'd say this is a viable option (given that this behavior shows up also in practice, but why shouldn't it).

@kohr-h kohr-h mentioned this issue Jul 6, 2017
8 tasks
kohr-h pushed a commit to kohr-h/odl that referenced this issue Sep 26, 2017
@kohr-h kohr-h closed this as completed in 241e344 Sep 26, 2017
kohr-h pushed a commit that referenced this issue Sep 26, 2017
MAINT: unify usage of calls to parent classes, closes #1061
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants