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

speed up isinstance and issubclass for the usual cases #66730

Closed
birkenfeld opened this issue Oct 2, 2014 · 7 comments
Closed

speed up isinstance and issubclass for the usual cases #66730

birkenfeld opened this issue Oct 2, 2014 · 7 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@birkenfeld
Copy link
Member

BPO 22540
Nosy @birkenfeld, @ncoghlan, @pitrou, @vstinner, @1st1
Files
  • pyobject_issubclass_isinstance_speedup.patch
  • pyobject_issubclass_isinstance_speedup_v2.patch
  • 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 2014-10-04.07:07:00.198>
    created_at = <Date 2014-10-02.11:39:12.578>
    labels = ['interpreter-core', 'performance']
    title = 'speed up isinstance and issubclass for the usual cases'
    updated_at = <Date 2014-10-04.07:07:00.198>
    user = 'https://github.com/birkenfeld'

    bugs.python.org fields:

    activity = <Date 2014-10-04.07:07:00.198>
    actor = 'georg.brandl'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-10-04.07:07:00.198>
    closer = 'georg.brandl'
    components = ['Interpreter Core']
    creation = <Date 2014-10-02.11:39:12.578>
    creator = 'georg.brandl'
    dependencies = []
    files = ['36781', '36789']
    hgrepos = []
    issue_num = 22540
    keywords = ['patch']
    message_count = 7.0
    messages = ['228218', '228306', '228308', '228311', '228313', '228317', '228339']
    nosy_count = 6.0
    nosy_names = ['georg.brandl', 'ncoghlan', 'pitrou', 'vstinner', 'python-dev', 'yselivanov']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue22540'
    versions = ['Python 3.5']

    @birkenfeld
    Copy link
    Member Author

    With the introduction of ABCs, PyObject_IsInstance (and for this issue, everything is also valid for PyObject_IsSubclass) has to check for a type's __instancecheck__ before falling back to the built-in behavior.

    However, the "type" type has an __instancecheck__ method (that calls the built-in behavior too), probably for consistency on the Python side.

    This means that the fast path is never taken, and every isinstance() call is slowed down unnecessarily.

    This patch introduces a new fast path by checking for PyType_Type exactly before looking up the __instancecheck__.

    It also introduces a check for exact match in PyObject_IsSubclass() analogous to one that is already present in PyObject_IsInstance(). Note that this will break __subclasscheck__ implementations that would deny issubclass(C, C).

    This patch is not only useful for speeding up isinstance() and issubclass() calls, but also has other effects such as not slowing down the improvement of issue bpo-12029.

    @birkenfeld birkenfeld added interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Oct 2, 2014
    @birkenfeld
    Copy link
    Member Author

    Addressing review comments.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 3, 2014

    New changeset 4f33a4a2b425 by Georg Brandl in branch 'default':
    Closes bpo-22540: speed up PyObject_IsInstance and PyObject_IsSubclass in the common case that the second argument has metaclass "type".
    https://hg.python.org/cpython/rev/4f33a4a2b425

    @python-dev python-dev mannequin closed this as completed Oct 3, 2014
    @vstinner
    Copy link
    Member

    vstinner commented Oct 3, 2014

    A buildbot is failing since your change.

    http://buildbot.python.org/all/builders/AMD64%20Debian%20root%203.x/builds/1261/steps/test/logs/stdio

    ======================================================================
    ERROR: test_decimal_fraction_comparison (test.test_decimal.CUsabilityTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_decimal.py", line 1711, in test_decimal_fraction_comparison
        self.assertLess(D(0), F(1,9999999999999999999999999999999999999))
      File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/case.py", line 1174, in assertLess
        if not a < b:
    TypeError: unorderable types: decimal.Decimal() < Fraction()

    ======================================================================
    FAIL: test_abc (test.test_decimal.CPythonAPItests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_decimal.py", line 2404, in test_abc
        self.assertTrue(issubclass(Decimal, numbers.Number))
    AssertionError: False is not true

    ======================================================================
    ERROR: test_mixed_comparisons (test.test_numeric_tower.ComparisonTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_numeric_tower.py", line 173, in test_mixed_comparisons
        self.assertLess(first, second)
      File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/case.py", line 1174, in assertLess
        if not a < b:
    TypeError: unorderable types: decimal.Decimal() < Fraction()

    @vstinner vstinner reopened this Oct 3, 2014
    @pitrou
    Copy link
    Member

    pitrou commented Oct 3, 2014

    The test_decimal failure can be reproduced using:

    ./python -m test -W test_datetime test_decimal

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 3, 2014

    New changeset 6cfe929d1de5 by Antoine Pitrou in branch 'default':
    Make test_datetime a better citizen (issue bpo-22540)
    https://hg.python.org/cpython/rev/6cfe929d1de5

    @birkenfeld
    Copy link
    Member Author

    Thanks for fixing! I did run "make test" locally, but that did not produce the failure.

    @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