-
-
Notifications
You must be signed in to change notification settings - Fork 31.2k
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
bpo-46841: Use inline caching for calls #31709
Conversation
🤖 New build scheduled with the buildbot fleet by @markshannon for commit 2021895 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
🤖 New build scheduled with the buildbot fleet by @markshannon for commit 2021895 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
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.
Excellent.
A couple comments, but nothing to block merging.
); | ||
} | ||
/* Maximum size of code to quicken, in code units. */ | ||
#define MAX_SIZE_TO_QUICKEN 10000 |
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.
Is this just to get the unpack sequence benchmark to work again, or something else?
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.
Nope, just for unpack_sequence
.
PyObject *isinstance; | ||
PyObject *len; | ||
PyObject *list_append; | ||
}; |
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 think the existence of PyList_Type
as part of the API means that list.append
must be per-process unique.
In other words, list_append
could be static.
I'm happy to leave it as is for now, though. We should look to make the whole struct static, although the mutability of builtin functions makes that tricky for isinstance
and len
.
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 believe each interpreter has its own builtins
module (check out _PyBuiltin_Init
), so making this static could be tricky. As you said, though: probably worth looking into in the future.
Also:
PRECALL
/CALL
specialization statsPRECALL
/CALL
instructionsNext steps tracked at faster-cpython/ideas#310.
https://bugs.python.org/issue46841