GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-131798: Add `MATCH_SEQUENCE/MAPPING` to the JIT optimizer by Sacul0457 · Pull Request #148124 · python/cpython

Conversation

Sacul0457 commented Apr 5, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

I hope there aren't any edge cases that I did not consider...

Issue: Better uop coverage in the JIT optimizer #131798

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          optimize MATCH_SEQUENCE/MAPPING

16f6ab0

Sacul0457 requested review from Fidget-Spinner, markshannon, savannahostrowski and tomasr8 as code owners

April 5, 2026 14:12

bedevere-app Bot added the awaiting review label

Apr 5, 2026

bedevere-app Bot mentioned this pull request

Apr 5, 2026

Better uop coverage in the JIT optimizer #131798

Open


          add else case to set the type to PyBool_Type

c366767

eendebakpt added the skip news label

Apr 5, 2026

cocolato reviewed

Apr 7, 2026

View reviewed changes

Comment thread

Python/optimizer_bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/optimizer_bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/optimizer_bytecodes.c

		op(_MATCH_MAPPING, (subject -- subject, res)) {
		if (sym_has_type(subject)) {
		PyTypeObject *type = sym_get_type(subject);
		int match = type->tp_flags & Py_TPFLAGS_MAPPING;

cocolato Apr 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wonder if this logic will be broken by abc.register.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Sacul0457 Apr 7, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I'm not too sure either. I'll try to see if I can find out more

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

cocolato Apr 12, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

abc.register will change the tp_flags but don't update the type version, so JIT won't invalid the old trace

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

cocolato Apr 12, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fidget-Spinner Do we need call assign_version_tag when use abc.register?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

kumaraditya303 Apr 12, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? ABC never changes the actual type flags.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Fidget-Spinner Apr 12, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

abc.register will change the tp_flags but don't update the type version, so JIT won't invalid the old trace

That sounds like a bug in abc.register. We should fix abc.register as that might affect the specializing interpreter as well.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

kumaraditya303 Apr 12, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I thought it already did invalidation, so yes this is a bug.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cocolato reacted with heart emoji

All reactions

❤️ 1 reaction

Fidget-Spinner Apr 12, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cocolato do you mind opening an issue for abc.register separately please?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

cocolato Apr 12, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, i will try to create the issue and solve this :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Fidget-Spinner Apr 12, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to solve it (unless you want to of course), but just opening an issue is fine :). Thank you

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cocolato reacted with heart emoji

All reactions

❤️ 1 reaction


          address review

a405ca7

kumaraditya303 approved these changes

Apr 12, 2026

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels

Apr 12, 2026

cocolato mentioned this pull request

Apr 12, 2026

abc.register changes tp_flags without updating type_version #148450

Closed

github-actions Bot commented May 18, 2026

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot added the stale Stale PR or inactive for long period of time. label

May 18, 2026

markshannon requested changes

May 21, 2026

View reviewed changes

markshannon left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we know the type, we can optimize the code as well as refining the type information

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Python/optimizer_bytecodes.c Outdated

		PyTypeObject *type = sym_get_type(subject);
		if (type != NULL) {
		int match = type->tp_flags & Py_TPFLAGS_MAPPING;
		res = sym_new_const(ctx, match ? Py_True : Py_False);

markshannon May 21, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we know what _MATCH_MAPPING is going to produce, we can do this:

REPLACE_OP(this_instr, _LOAD_COMMON_CONST, match? CONSTANT_TRUE : CONSTANT_FALSE, 0);

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Python/optimizer_bytecodes.c Outdated

		PyTypeObject *type = sym_get_type(subject);
		if (type != NULL) {
		int match = type->tp_flags & Py_TPFLAGS_SEQUENCE;
		res = sym_new_const(ctx, match ? Py_True : Py_False);

markshannon May 21, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise:

REPLACE_OP(this_instr, _LOAD_COMMON_CONST, match? CONSTANT_TRUE : CONSTANT_FALSE, 0);

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot added awaiting changes and removed awaiting merge labels

May 21, 2026

bedevere-app Bot commented May 21, 2026

Copy link

Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sacul0457 added 3 commits

May 21, 2026 18:57


          address review: use replace op instead

e167770


          Merge branch 'main' into Optimize-MATCH_SEQUENCE-MAPPING

ba91a21


          use _LOAD_COMMON_CONSTANT and update tests

bd2a9d8

Sacul0457 commented May 21, 2026

Copy link

Copy Markdown

Contributor Author

I have made the requested changes; please review again

I believe mark meant _LOAD_COMMON_CONSTANT? And I have updated the tests accordingly.
However, there seems to be an error during runtime for MATCH_MAPPING when I run that test, not sure why. Feel free to edit the PR accordingly, thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot added awaiting change review and removed awaiting changes labels

May 21, 2026

bedevere-app Bot commented May 21, 2026

Copy link

Copy Markdown

Thanks for making the requested changes!

@markshannon, @kumaraditya303: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot requested review from kumaraditya303 and markshannon

May 21, 2026 11:25


          Merge branch 'main' into Optimize-MATCH_SEQUENCE-MAPPING

e178f70

kumaraditya303 reviewed

May 23, 2026

View reviewed changes

Comment thread

Python/optimizer_bytecodes.c

		PyTypeObject *type = sym_get_type(subject);
		if (type != NULL) {
		int match = type->tp_flags & Py_TPFLAGS_MAPPING;
		REPLACE_OP(this_instr, _LOAD_COMMON_CONSTANT, match? CONSTANT_TRUE : CONSTANT_FALSE, 0);

kumaraditya303 May 23, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to assign to res here as well

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Sacul0457 May 27, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added, sorry for the wait

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

github-actions Bot removed the stale Stale PR or inactive for long period of time. label

May 26, 2026


          address review: assign res

1e25f00

kumaraditya303 reviewed

May 27, 2026

View reviewed changes

Comment thread

Python/optimizer_bytecodes.c

		PyTypeObject *type = sym_get_type(subject);
		if (type != NULL) {
		int match = type->tp_flags & Py_TPFLAGS_SEQUENCE;
		REPLACE_OP(this_instr, _LOAD_COMMON_CONSTANT, match? CONSTANT_TRUE : CONSTANT_FALSE, 0);

kumaraditya303 May 27, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other places, LOAD_CONST_INLINE_BORROW is used which would be faster

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

github-actions Bot commented Aug 30, 2026

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot added the stale Stale PR or inactive for long period of time. label

Aug 30, 2026

Reviewers

cocolato

cocolato left review comments

kumaraditya303

kumaraditya303 approved these changes

tomasr8

Awaiting requested review from tomasr8

tomasr8 is a code owner

Fidget-Spinner

Awaiting requested review from Fidget-Spinner

Fidget-Spinner is a code owner

savannahostrowski

Awaiting requested review from savannahostrowski

markshannon

Awaiting requested review from markshannon

markshannon is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting change review skip news stale

Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Sacul0457 commented Apr 5, 2026 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

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!

Fidget-Spinner Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 18, 2026

Uh oh!

markshannon 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!

bedevere-app Bot commented May 21, 2026

Uh oh!

Sacul0457 commented May 21, 2026

Uh oh!

bedevere-app Bot commented May 21, 2026

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!

github-actions Bot commented Aug 30, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Sacul0457 commented Apr 5, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Fidget-Spinner Apr 12, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.