Skip to content

bpo-45829: Specialize BINARY_SUBSCR for __getitem__ implemented in Python. #29592

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

Merged
merged 7 commits into from
Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Include/internal/pycore_code.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ typedef struct {
uint8_t original_oparg;
uint8_t counter;
uint16_t index;
uint32_t version;
} _PyAdaptiveEntry;


Expand Down Expand Up @@ -266,7 +267,7 @@ int _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name
int _Py_Specialize_StoreAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache);
int _Py_Specialize_LoadGlobal(PyObject *globals, PyObject *builtins, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache);
int _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache);
int _Py_Specialize_BinarySubscr(PyObject *sub, PyObject *container, _Py_CODEUNIT *instr);
int _Py_Specialize_BinarySubscr(PyObject *sub, PyObject *container, _Py_CODEUNIT *instr, SpecializedCacheEntry *cache);
int _Py_Specialize_CallFunction(PyObject *callable, _Py_CODEUNIT *instr, int nargs, SpecializedCacheEntry *cache, PyObject *builtins);
void _Py_Specialize_BinaryOp(PyObject *lhs, PyObject *rhs, _Py_CODEUNIT *instr,
SpecializedCacheEntry *cache);
Expand Down
65 changes: 33 additions & 32 deletions Include/opcode.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Lib/opcode.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ def jabs_op(name, op):
"BINARY_OP_SUBTRACT_INT",
"BINARY_OP_SUBTRACT_FLOAT",
"BINARY_SUBSCR_ADAPTIVE",
"BINARY_SUBSCR_GETITEM",
"BINARY_SUBSCR_LIST_INT",
"BINARY_SUBSCR_TUPLE_INT",
"BINARY_SUBSCR_DICT",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Specialize :opcode:`BINARY_SUBSCR` for classes with a ``__getitem__`` method
implemented in Python
61 changes: 38 additions & 23 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -2140,21 +2140,21 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
}

TARGET(BINARY_SUBSCR_ADAPTIVE) {
if (oparg == 0) {
SpecializedCacheEntry *cache = GET_CACHE();
if (cache->adaptive.counter == 0) {
PyObject *sub = TOP();
PyObject *container = SECOND();
next_instr--;
if (_Py_Specialize_BinarySubscr(container, sub, next_instr) < 0) {
if (_Py_Specialize_BinarySubscr(container, sub, next_instr, cache) < 0) {
goto error;
}
DISPATCH();
}
else {
STAT_INC(BINARY_SUBSCR, deferred);
// oparg is the adaptive cache counter
UPDATE_PREV_INSTR_OPARG(next_instr, oparg - 1);
assert(_Py_OPCODE(next_instr[-1]) == BINARY_SUBSCR_ADAPTIVE);
assert(_Py_OPARG(next_instr[-1]) == oparg - 1);
cache->adaptive.counter--;
assert(cache->adaptive.original_oparg == 0);
/* No need to set oparg here; it isn't used by BINARY_SUBSCR */
STAT_DEC(BINARY_SUBSCR, unquickened);
JUMP_TO_INSTRUCTION(BINARY_SUBSCR);
}
Expand Down Expand Up @@ -2223,6 +2223,37 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
DISPATCH();
}

TARGET(BINARY_SUBSCR_GETITEM) {
PyObject *sub = TOP();
PyObject *container = SECOND();
SpecializedCacheEntry *caches = GET_CACHE();
_PyAdaptiveEntry *cache0 = &caches[0].adaptive;
_PyObjectCache *cache1 = &caches[-1].obj;
PyFunctionObject *getitem = (PyFunctionObject *)cache1->obj;
DEOPT_IF(Py_TYPE(container)->tp_version_tag != cache0->version, BINARY_SUBSCR);
DEOPT_IF(getitem->func_version != cache0->index, BINARY_SUBSCR);
PyCodeObject *code = (PyCodeObject *)getitem->func_code;
size_t size = code->co_nlocalsplus + code->co_stacksize + FRAME_SPECIALS_SIZE;
assert(code->co_argcount == 2);
InterpreterFrame *new_frame = _PyThreadState_BumpFramePointer(tstate, size);
if (new_frame == NULL) {
goto error;
}
_PyFrame_InitializeSpecials(new_frame, PyFunction_AS_FRAME_CONSTRUCTOR(getitem),
NULL, code->co_nlocalsplus);
STACK_SHRINK(2);
new_frame->localsplus[0] = container;
new_frame->localsplus[1] = sub;
for (int i = 2; i < code->co_nlocalsplus; i++) {
new_frame->localsplus[i] = NULL;
}
_PyFrame_SetStackPointer(frame, stack_pointer);
new_frame->previous = frame;
frame = cframe.current_frame = new_frame;
new_frame->depth = frame->depth + 1;
goto start_frame;
}

TARGET(LIST_APPEND) {
PyObject *v = POP();
PyObject *list = PEEK(oparg);
Expand Down Expand Up @@ -4878,29 +4909,13 @@ opname ## _miss: \
JUMP_TO_INSTRUCTION(opname); \
}

#define MISS_WITH_OPARG_COUNTER(opname) \
opname ## _miss: \
{ \
STAT_INC(opname, miss); \
uint8_t oparg = _Py_OPARG(next_instr[-1])-1; \
UPDATE_PREV_INSTR_OPARG(next_instr, oparg); \
assert(_Py_OPARG(next_instr[-1]) == oparg); \
if (oparg == 0) /* too many cache misses */ { \
oparg = ADAPTIVE_CACHE_BACKOFF; \
next_instr[-1] = _Py_MAKECODEUNIT(opname ## _ADAPTIVE, oparg); \
STAT_INC(opname, deopt); \
} \
STAT_DEC(opname, unquickened); \
JUMP_TO_INSTRUCTION(opname); \
}

MISS_WITH_CACHE(LOAD_ATTR)
MISS_WITH_CACHE(STORE_ATTR)
MISS_WITH_CACHE(LOAD_GLOBAL)
MISS_WITH_CACHE(LOAD_METHOD)
MISS_WITH_CACHE(CALL_FUNCTION)
MISS_WITH_CACHE(BINARY_OP)
MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR)
MISS_WITH_CACHE(BINARY_SUBSCR)
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing it's still worth keeping the logic for oparg counters, just in case we implement a new family that can uses it in the future?

Copy link
Member Author

Choose a reason for hiding this comment

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

Might as well delete it. It's in the git history.


binary_subscr_dict_error:
{
Expand Down
16 changes: 8 additions & 8 deletions Python/opcode_targets.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading