[issue1762] Inheriting from ABC slows Decimal down.

[Raymond Hettinger]

Raymond Hettinger added the comment:

Whoa!  I thought we had arrived at a decision to leave decimal alone.  
Please do not infect this module with numbers.py.

--
nosy: +rhettinger

__
Tracker <[EMAIL PROTECTED]>

__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1762] Inheriting from ABC slows Decimal down.

[Jeffrey Yasskin]

Jeffrey Yasskin added the comment:

I measured various implementations of __instancecheck__ using
`./python.exe -m timeit -s 'from rational import Rational; r =
Rational(3, 2)' '...'` on my 2.33 GHz MacBook, with ... replaced by
either isinstance(r, Rational) or isinstance(3, Rational) to measure
both the positive and negative cases. The big win comes from avoiding
the genexp and the set. Then we win smaller amounts by being more
careful about avoiding extra calls to __subclasscheck__ and by inlining
the cache checks.

# Current code
return any(cls.__subclasscheck__(c)
   for c in set([instance.__class__, type(instance)]))
isinstance(3, Rational): 4.65 usec
isinstance(r, Rational): 7.47 usec

# The best we can do simply in Python
return cls.__subclasscheck__(instance.__class__)
isinstance(3, Rational): 2.08 usec
isinstance(r, Rational): 1.72 usec

# Preserve behavior, simply
return (cls.__subclasscheck__(instance.__class__) or
cls.__subclasscheck__(type(instance)))
isinstance(3, Rational): 3.03 usec
isinstance(r, Rational): 1.8 usec

# Preserve behavior, complexly
ic = instance.__class__
if cls.__subclasscheck__(ic):
return True
t = type(instance)
return t is not ic and cls.__subclasscheck__(t)
isinstance(3, Rational): 2.38 usec
isinstance(r, Rational): 1.86 usec

# Inlined for new-style classes
subclass = instance.__class__
if subclass in cls._abc_cache:
return True
type_ = type(instance)
if type_ is subclass:
if (cls._abc_negative_cache_version ==
ABCMeta._abc_invalidation_counter and
subclass in cls._abc_negative_cache):
return False
return cls.__subclasscheck__(subclass)
return (cls.__subclasscheck__(subclass) or
cls.__subclasscheck__(type_))
isinstance(3, Rational): 2.26 usec
isinstance(r, Rational): 1.49 usec

__
Tracker <[EMAIL PROTECTED]>

__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1762] Inheriting from ABC slows Decimal down.

[Christian Heimes]

Christian Heimes added the comment:

without abc:
$ time ./python Lib/test/regrtest.py test_decimal
real0m10.113s
user0m9.685s
sys 0m0.196s

with numbers.Real subclass:
$ time ./python Lib/test/regrtest.py  test_decimal
real0m16.232s
user0m15.241s
sys 0m0.384s

Proposed patch:
$ time ./python Lib/test/regrtest.py  test_decimal
real0m11.128s
user0m9.533s
sys 0m0.260s

Only with if instance.__class__ in cls._abc_cache: return True
$ time ./python Lib/test/regrtest.py  test_decimal
real0m11.201s
user0m10.345s
sys 0m0.292s

Wow, __instancecheck__ must be called a *lot* of times.

__
Tracker <[EMAIL PROTECTED]>

__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1762] Inheriting from ABC slows Decimal down.

[Christian Heimes]

Christian Heimes added the comment:

The patch implements the rich cmp slots for <, <=, > and >= required for
numbers.Real.

Added file: http://bugs.python.org/file9107/trunk_decimal_richcmp.patch

__
Tracker <[EMAIL PROTECTED]>

__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1762] Inheriting from ABC slows Decimal down.

[Guido van Rossum]

Guido van Rossum added the comment:

What change is responsible for the speedup? The cache check or removing
type(instance) from the set?  Since type(instance) is usually the same
object as instance.__class__, the set will still have only one element
(in particular for Decimal this is the case) so I don't see why that
change is necessary.

It's also important to find out where __instancecheck__ is called; I
don't see where this is happening, so I suspect it's probably somewhere
internal.

__
Tracker <[EMAIL PROTECTED]>

__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1762] Inheriting from ABC slows Decimal down.

[Christian Heimes]

Christian Heimes added the comment:

__instancecheck__ contains unnecessary code. If we restrict ABCs to new
style classes we could make the function faster:

Old:

def __instancecheck__(cls, instance):
"""Override for isinstance(instance, cls)."""
return any(cls.__subclasscheck__(c)
   for c in {instance.__class__, type(instance)})
New:

def __instancecheck__(cls, instance):
"""Override for isinstance(instance, cls)."""
# safe a function call for common case
if instance.__class__ in cls._abc_cache:
return True
return cls.__subclasscheck__(instance.__class__)

--
assignee:  -> gvanrossum
nosy: +gvanrossum

__
Tracker <[EMAIL PROTECTED]>

__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1762] Inheriting from ABC slows Decimal down.

[Jeffrey Yasskin]

Jeffrey Yasskin added the comment:

I've only verified the behavior on 2.6, but I suspect it's true for both.

__
Tracker <[EMAIL PROTECTED]>

__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1762] Inheriting from ABC slows Decimal down.

[Christian Heimes]

Christian Heimes added the comment:

Is this for 2.6 or 3.0?

--
nosy: +tiran
priority:  -> high

__
Tracker <[EMAIL PROTECTED]>

__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1762] Inheriting from ABC slows Decimal down.

[Jeffrey Yasskin]

New submission from Jeffrey Yasskin:

Adding numbers.Real to Decimal's base classes almost doubles the time
its its test suite takes to run. A profile revealed that a large
fraction of that slowdown was in __instancecheck__, but even after
optimizing that, it's still about 25% slower. It looks like the rest of
the slowdown is still in other parts of the isinstance() check. It would
be nice if inheriting from ABCs didn't slow your class down.

--
components: Library (Lib)
messages: 59543
nosy: jyasskin, nnorwitz
severity: normal
status: open
title: Inheriting from ABC slows Decimal down.

__
Tracker <[EMAIL PROTECTED]>

__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com