Skip to content

gh-131798: fold super method lookups in JIT - #148231

Merged
kumaraditya303 merged 10 commits into
python:mainfrom
kumaraditya303:jit-super
Apr 9, 2026
Merged

gh-131798: fold super method lookups in JIT#148231
kumaraditya303 merged 10 commits into
python:mainfrom
kumaraditya303:jit-super

Conversation

@kumaraditya303

@kumaraditya303kumaraditya303 commented Apr 7, 2026•
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

@bedevere-appbedevere-appBot mentioned this pull request Apr 7, 2026
Open
@Fidget-SpinnerFidget-Spinner changed the title gh-131798: cache super method lookups in JITgh-131798: fold super method lookups in JITApr 7, 2026
@Fidget-Spinner

Fidget-Spinner commented Apr 8, 2026

Copy link
Copy Markdown
Member

I see a 10% speedup on richards_super with this PR!

Mean +- std dev: [richards_super_main] 28.8 ms +- 0.5 ms -> [richards_super_fold] 26.1 ms +- 0.5 ms: 1.10x faster

Fidget-Spinner
Fidget-Spinner reviewed Apr 8, 2026
View reviewed changes

@Fidget-SpinnerFidget-Spinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, just one stylistic nit. Thanks!

Comment threadPython/optimizer_analysis.c
return sym_new_not_null(ctx);
}
/* Normal case: obj_type must be a subtype of su_type */
if (!PyType_IsSubtype(obj_type, su_type)) {

@Fidget-SpinnerFidget-SpinnerApr 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note to self: this is not same as subclasscheck/instancecheck, which can call arbitrary code.

Comment threadPython/optimizer_analysis.c Outdated
@Fidget-Spinner

Fidget-Spinner commented Apr 8, 2026

Copy link
Copy Markdown
Member

Please fix the merge conflicts. LOAD_CONST_UNDER_INLINE_BORROW was removed as it's a duplicate of _INSERT_1_LOAD_CONST_INLIE_BORROW #148235

@kumaraditya303

kumaraditya303 commented Apr 8, 2026

Copy link
Copy Markdown
ContributorAuthor

Please fix the merge conflicts.

Fixed.

Fidget-Spinner
Fidget-Spinner approved these changes Apr 8, 2026
View reviewed changes

@Fidget-SpinnerFidget-Spinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks. This LGTM.

Comment threadPython/optimizer_analysis.c
_Py_BloomFilter_Add(dependencies, su_type);
PyType_Watch(TYPE_WATCHER_ID, (PyObject *)obj_type);
_Py_BloomFilter_Add(dependencies, obj_type);
return sym_new_const_steal(ctx, lookup);

@Fidget-SpinnerFidget-SpinnerApr 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note to self: need steal version because _PySuper_Lookup returns a strong reference, unlike _PyType_Lookup

@Fidget-Spinner

Fidget-Spinner commented Apr 8, 2026

Copy link
Copy Markdown
Member

Please pull in main and regen cases. Thank you.

@markshannon

markshannon commented Apr 8, 2026

Copy link
Copy Markdown
Member

Can we have a test for the case where the superclass changes after optimization, to make sure that we de-opt properly?

@kumaraditya303

kumaraditya303 commented Apr 8, 2026

Copy link
Copy Markdown
ContributorAuthor

Added test and fixed merge conflicts.

Fidget-Spinner
Fidget-Spinner approved these changes Apr 9, 2026
View reviewed changes

@Fidget-SpinnerFidget-Spinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding the test

@kumaraditya303
kumaraditya303 merged commit 458aca9 into python:mainApr 9, 2026
74 checks passed
@kumaraditya303
kumaraditya303 deleted the jit-super branch April 9, 2026 07:55
ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
@kumaraditya303@ljfp
pythongh-131798: fold super method lookups in JIT (python#148231)
13a0ac6
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fidget-SpinnerFidget-SpinnerFidget-Spinner approved these changes
@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner
@tomasr8tomasr8Awaiting requested review from tomasr8tomasr8 is a code owner
@savannahostrowskisavannahostrowskiAwaiting requested review from savannahostrowskisavannahostrowski is a code owner

Assignees

No one assigned

Labels

skip newstopic-JIT

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kumaraditya303@Fidget-Spinner@markshannon