gh-131798: JIT: Further optimize `_CALL_ISINSTANCE` for class tuples by tomasr8 · Pull Request #134543 · python/cpython

tomasr8 · 2025-05-22T20:31:47Z

We can already const eval isinstance(inst, cls) calls when both arguments are known, but only if cls is a single class (e.g. int).
This PR adds support for isinstance(inst, (cls1, cls2, ..., clsN)). This allows us to optimize for example:

isinstance(42, (int, str)) (to True)
isinstance(42, (bool, str)) (to False)

We can narrow to True even when only some of the classes are known, as long as inst is an instance of one of the known classes.

Issue: Better uop coverage in the JIT optimizer #131798

tomasr8 · 2025-05-22T20:34:13Z

+ self.assertIn("_BUILD_TUPLE", uops)
+ self.assertIn("_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW", uops)


_BUILD_TUPLE is preventing us from optimizing out _POP_CALL_TWO_LOAD_CONST_INLINE_BORROW.
The bytecode is basically:
LOAD_CONSTLOAD_CONST_BUILD_TUPLE_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW
To optimize this, we'd need some special handling for _BUILD_TUPLE in remove_unneeded_uops.

Got it, might be worth looking into next if you're up for it! Could be tricky, though.

bedevere-app · 2025-07-01T21:42:38Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

tomasr8 · 2025-07-19T09:38:35Z

I'll fix the PR hopefully this weekend.

So that was a lie 😆

Anyway, I think I addressed all your comments! :)

I have made the requested changes; please review again

bedevere-app · 2025-07-19T09:38:40Z

Thanks for making the requested changes!

@brandtbucher: please review the changes made to this pull request.

brandtbucher

Looks good, just a few suggestions:

brandtbucher · 2025-07-29T02:12:52Z

+ self.assertIn("_BUILD_TUPLE", uops)
+ self.assertIn("_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW", uops)


Got it, might be worth looking into next if you're up for it! Could be tricky, though.

brandtbucher · 2025-07-29T02:13:45Z

+ for _ in range(n):
+ # One of the classes is unknown, so we can't narrow
+ # to True or False, only bool
+ y = isinstance(42, (eval('str'), int))


I'm confused, why can't we narrow to True? We can't remove the call, but the result is known.

Yep, that was a brainfart 😄 I somehow conflated narrowing to True/False with replacing the op, but we can obviously just narrow without removing the call as you said!

brandtbucher · 2025-07-29T02:15:11Z

+ def testfunc(n):
+ x = 0
+ for _ in range(n):
+ y = isinstance(42, ())


I had to pause and think for a second to figure out what this would even do. Nice edge case.

brandtbucher · 2025-07-29T02:18:11Z

+ // It could potentially define its own __instancecheck__
+ // method so we can only deduce bool.
+ out = NULL;
+ break;


Could this be continue? We could still narrow to True later to handle cases like the one I called out above, right?
Suggested change
break;
continue;

Yup, see #134543 (comment). I just needed to add some extra bookkeeping to know when we can replace the call and when not.

brandtbucher · 2025-07-29T02:18:54Z

+ }
+ PyTypeObject *cls_o = (PyTypeObject *)sym_get_const(ctx, item);
+ if (cls_o &&
+ sym_matches_type(item, &PyType_Type) &&


Maybe add a comment explaining that this is to protect against metaclasses definine __instancecheck__.

How would you formulate it? I don't think of it as specifically a guard for __instancecheck__ but basically PyObject_TypeCheck adapted to the JIT optimizer.

We're not only checking that the object is a subclass of type, we're also checking that it is an exact instance of type itself. We care about this second condition because it guarantees that __instancecheck__ doesn't exist (otherwise we would need to look it up to check if it exists or not).

Thanks for the clarification, I added a comment :)

brandtbucher · 2025-07-29T20:39:38Z

Off-topic, but something I've been thinking about that you may want to check out next: removing bounds checks when indexing into strings and tuples with a constant index.

tomasr8 · 2025-08-02T19:38:42Z

Thanks for the review! Super helpful as always :) I'll leave this open for a few days so @Fidget-Spinner can also have a look :)

Off-topic, but something I've been thinking about that you may want to check out next: removing bounds checks when indexing into strings and tuples with a constant index.

Sure, I can look into that! I guess this will also require knowing the string/tuple length?

Fidget-Spinner · 2025-08-20T14:33:36Z

+ if (cls_o &&
+ // Ensure that item is an exact instance of `type` ensuring that
+ // there is no __instancecheck__ defined.
+ sym_matches_type(item, &PyType_Type) &&


Wait, wouldn't this only work on something that is isinstance(x, type), like the actual type itself, not any type like int? Sorry, I'm confused!

Hmm I don't believe so. If item is e.g. int then sym_matches_type(item, &PyType_Type) should return true

For example something like this seems to work:
JitOptRefref_type=_Py_uop_sym_new_const(ctx, (PyObject*)&PyLong_Type); TEST_PREDICATE(_Py_uop_sym_matches_type(ref_type, &PyType_Type), "int is not a type");

That''s kinda strange. The code for _Py_uop_sym_matches_type is:
_Py_uop_sym_get_type(sym) == typ
so it's a pointer comparison, not a subclass check.

The subclass check is done after with PyType_IsSubtype if that's what you mean? The current version of _CALL_ISINSTANCE (which does not handle tuples) works the same way though. @brandtbucher, what do you think?

Fidget-Spinner · 2025-12-28T17:48:13Z

I'm really sorry Tomas, but the more I look at the code, the more I feel the complexity is not worth it in this case. __instancecheck__ is tricky (and you've handled that well in the code), but I feel for the additional lines we get in the optimizer_bytecodes.c, we don't get much back in terms of optimization :(.

Fidget-Spinner

We need a test for a type with __instancecheck__, otherwise it LGTM, and you can merge after the test is added

github-actions · 2026-04-25T06:30:32Z

This PR is stale because it has been open for 30 days with no activity.

tomasr8 added 2 commits May 22, 2025 22:20

Add news entry
1d582e8

Optimize _CALL_ISINSTANCE for class tuples
97aca36

tomasr8 changed the title ~~gh-134369: JIT: Further optimize _CALL_ISINSTANCE for class tuples~~gh-131798: JIT: Further optimize _CALL_ISINSTANCE for class tuplesMay 22, 2025

bedevere-appBot mentioned this pull request May 22, 2025
Better uop coverage in the JIT optimizer #131798
Open

Add more tests
467bcb9

tomasr8 commented May 22, 2025
View reviewed changes

tomasr8 marked this pull request as ready for review May 22, 2025 22:34

tomasr8 requested a review from Fidget-Spinner as a code owner May 22, 2025 22:34

bedevere-appBot added the awaiting review label May 22, 2025

tomasr8 requested a review from brandtbucher May 22, 2025 22:34

brandtbucher requested changes Jul 1, 2025
View reviewed changes

Comment threadPython/optimizer_bytecodes.c
Comment threadPython/optimizer_bytecodes.c Outdated

bedevere-appBot added awaiting changes and removed awaiting review labels Jul 1, 2025

tomasr8 added 5 commits July 5, 2025 23:17

Bail when encountering an unknown class
d2e339f

Remove an extra test case
0465c9a

Simplify code
088ccd8

Merge branch 'main' into jit-tuple-isinstance
fb28e06

Fix incorrect type
1e9b95f

bedevere-appBot added awaiting change review and removed awaiting changes labels Jul 19, 2025

bedevere-appBot requested a review from brandtbucher July 19, 2025 09:38

brandtbucher approved these changes Jul 29, 2025
View reviewed changes

bedevere-appBot added awaiting merge and removed awaiting change review labels Jul 29, 2025

Narrow to true but keep the isinstance call
a09b860

tomasr8 requested a review from brandtbucher July 29, 2025 18:59

brandtbucher approved these changes Jul 29, 2025
View reviewed changes

Add a comment about __instancecheck__
17701d3

tomasr8 mentioned this pull request Aug 10, 2025
gh-131798: Remove bounds check when indexing into tuples with a constant index #137607
Merged

Fidget-Spinner reviewed Aug 20, 2025
View reviewed changes

tomasr8 added 2 commits December 27, 2025 22:33

Merge remote-tracking branch 'upstream/main' into jit-tuple-isinstance
cf71d8b

Regenerate files
25ae15f

tomasr8 requested a review from markshannon as a code owner December 27, 2025 21:34

tomasr8 closed this Dec 28, 2025

tomasr8 reopened this Jan 8, 2026

Fidget-Spinner reviewed Jan 9, 2026
View reviewed changes

github-actionsBot added the stale Stale PR or inactive for long period of time. label Apr 25, 2026

		self.assertIn("_BUILD_TUPLE", uops)
		self.assertIn("_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW", uops)

Uh oh!

Uh oh!

Conversation

tomasr8 commented May 22, 2025• edited by bedevere-app BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-appBot commented Jul 1, 2025

Uh oh!

tomasr8 commented Jul 19, 2025

Uh oh!

bedevere-appBot commented Jul 19, 2025

Uh oh!

brandtbucher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brandtbucher commented Jul 29, 2025

Uh oh!

tomasr8 commented Aug 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fidget-Spinner commented Dec 28, 2025

Uh oh!

Fidget-Spinner left a comment

Choose a reason for hiding this comment

Uh oh!

github-actionsBot commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tomasr8 commented May 22, 2025•
edited by bedevere-app Bot
Loading