Skip to content

bpo-46841: Don't use an oparg counter for STORE_SUBSCR #31742

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 2 commits into from
Mar 8, 2022
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
6 changes: 6 additions & 0 deletions Include/internal/pycore_code.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ typedef struct {

#define INLINE_CACHE_ENTRIES_PRECALL CACHE_ENTRIES(_PyPrecallCache)

typedef struct {
_Py_CODEUNIT counter;
} _PyStoreSubscrCache;

#define INLINE_CACHE_ENTRIES_STORE_SUBSCR CACHE_ENTRIES(_PyStoreSubscrCache)

/* Maximum size of code to quicken, in code units. */
#define MAX_SIZE_TO_QUICKEN 10000

Expand Down
1 change: 1 addition & 0 deletions Include/opcode.h

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

3 changes: 2 additions & 1 deletion Lib/importlib/_bootstrap_external.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ def _write_atomic(path, data, mode=0o666):
# STORE_ATTR)
# Python 3.11a5 3485 (Add an oparg to GET_AWAITABLE)
# Python 3.11a6 3486 (Use inline caching for PRECALL and CALL)
# Python 3.11a6 3487 (Remove the adaptive "oparg counter" mechanism)

# Python 3.12 will start with magic number 3500

Expand All @@ -408,7 +409,7 @@ def _write_atomic(path, data, mode=0o666):
# Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
# in PC/launcher.c must also be updated.

MAGIC_NUMBER = (3486).to_bytes(2, 'little') + b'\r\n'
MAGIC_NUMBER = (3487).to_bytes(2, 'little') + b'\r\n'
_RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little') # For import.c

_PYCACHE = '__pycache__'
Expand Down
2 changes: 1 addition & 1 deletion Lib/opcode.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def jabs_op(name, op, entries=0):
def_op('BEFORE_WITH', 53)
def_op('END_ASYNC_FOR', 54)

def_op('STORE_SUBSCR', 60)
def_op('STORE_SUBSCR', 60, 1)
def_op('DELETE_SUBSCR', 61)

def_op('GET_ITER', 68)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Modify :opcode:`STORE_SUBSCR` to use an inline cache entry (rather than its
oparg) as an adaptive counter.
29 changes: 9 additions & 20 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -2267,13 +2267,16 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
Py_DECREF(v);
Py_DECREF(container);
Py_DECREF(sub);
if (err != 0)
if (err != 0) {
goto error;
}
JUMPBY(INLINE_CACHE_ENTRIES_STORE_SUBSCR);
DISPATCH();
}

TARGET(STORE_SUBSCR_ADAPTIVE) {
if (oparg == 0) {
_PyStoreSubscrCache *cache = (_PyStoreSubscrCache *)next_instr;
if (cache->counter == 0) {
PyObject *sub = TOP();
PyObject *container = SECOND();
next_instr--;
Expand All @@ -2284,8 +2287,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
}
else {
STAT_INC(STORE_SUBSCR, deferred);
// oparg is the adaptive cache counter
UPDATE_PREV_INSTR_OPARG(next_instr, oparg - 1);
cache->counter--;
JUMP_TO_INSTRUCTION(STORE_SUBSCR);
}
}
Expand All @@ -2312,6 +2314,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
Py_DECREF(old_value);
Py_DECREF(sub);
Py_DECREF(list);
JUMPBY(INLINE_CACHE_ENTRIES_STORE_SUBSCR);
NOTRACE_DISPATCH();
}

Expand All @@ -2328,6 +2331,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
if (err != 0) {
goto error;
}
JUMPBY(INLINE_CACHE_ENTRIES_STORE_SUBSCR);
DISPATCH();
}

Expand Down Expand Up @@ -5520,21 +5524,6 @@ 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); \
} \
JUMP_TO_INSTRUCTION(opname); \
}

MISS_WITH_INLINE_CACHE(LOAD_ATTR)
MISS_WITH_INLINE_CACHE(STORE_ATTR)
MISS_WITH_INLINE_CACHE(LOAD_GLOBAL)
Expand All @@ -5545,7 +5534,7 @@ MISS_WITH_INLINE_CACHE(BINARY_OP)
MISS_WITH_INLINE_CACHE(COMPARE_OP)
MISS_WITH_INLINE_CACHE(BINARY_SUBSCR)
MISS_WITH_INLINE_CACHE(UNPACK_SEQUENCE)
MISS_WITH_OPARG_COUNTER(STORE_SUBSCR)
MISS_WITH_INLINE_CACHE(STORE_SUBSCR)

binary_subscr_dict_error:
{
Expand Down
14 changes: 7 additions & 7 deletions Python/specialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -301,12 +301,11 @@ optimize(_Py_CODEUNIT *instructions, int len)
uint8_t adaptive_opcode = adaptive_opcodes[opcode];
if (adaptive_opcode) {
instructions[i] = _Py_MAKECODEUNIT(adaptive_opcode, oparg);
int caches = _PyOpcode_InlineCacheEntries[opcode];
// Make sure the adaptive counter is zero:
assert((caches ? instructions[i + 1] : oparg) == 0);
assert(instructions[i + 1] == 0);
previous_opcode = -1;
previous_oparg = -1;
i += caches;
i += _PyOpcode_InlineCacheEntries[opcode];
}
else {
assert(!_PyOpcode_InlineCacheEntries[opcode]);
Expand Down Expand Up @@ -1313,14 +1312,15 @@ _Py_Specialize_BinarySubscr(
int
_Py_Specialize_StoreSubscr(PyObject *container, PyObject *sub, _Py_CODEUNIT *instr)
{
_PyStoreSubscrCache *cache = (_PyStoreSubscrCache *)(instr + 1);
PyTypeObject *container_type = Py_TYPE(container);
if (container_type == &PyList_Type) {
if (PyLong_CheckExact(sub)) {
if ((Py_SIZE(sub) == 0 || Py_SIZE(sub) == 1)
&& ((PyLongObject *)sub)->ob_digit[0] < (size_t)PyList_GET_SIZE(container))
{
*instr = _Py_MAKECODEUNIT(STORE_SUBSCR_LIST_INT,
initial_counter_value());
_Py_OPARG(*instr));
goto success;
}
else {
Expand All @@ -1338,8 +1338,7 @@ _Py_Specialize_StoreSubscr(PyObject *container, PyObject *sub, _Py_CODEUNIT *ins
}
}
if (container_type == &PyDict_Type) {
*instr = _Py_MAKECODEUNIT(STORE_SUBSCR_DICT,
initial_counter_value());
*instr = _Py_MAKECODEUNIT(STORE_SUBSCR_DICT, _Py_OPARG(*instr));
goto success;
}
#ifdef Py_STATS
Expand Down Expand Up @@ -1406,11 +1405,12 @@ _Py_Specialize_StoreSubscr(PyObject *container, PyObject *sub, _Py_CODEUNIT *ins
fail:
STAT_INC(STORE_SUBSCR, failure);
assert(!PyErr_Occurred());
*instr = _Py_MAKECODEUNIT(_Py_OPCODE(*instr), ADAPTIVE_CACHE_BACKOFF);
cache->counter = ADAPTIVE_CACHE_BACKOFF;
return 0;
success:
STAT_INC(STORE_SUBSCR, success);
assert(!PyErr_Occurred());
cache->counter = initial_counter_value();
return 0;
}

Expand Down