Skip to content
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

GH-93533: Shrink the LOAD_METHOD cache by one codeunit. #93537

Merged
merged 1 commit into from
Jun 7, 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
1 change: 0 additions & 1 deletion Include/internal/pycore_code.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ typedef struct {
typedef struct {
_Py_CODEUNIT counter;
_Py_CODEUNIT type_version[2];
_Py_CODEUNIT dict_offset;
_Py_CODEUNIT keys_version[2];
_Py_CODEUNIT descr[4];
} _PyLoadMethodCache;
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_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 @@ -407,6 +407,7 @@ def _write_atomic(path, data, mode=0o666):
# Python 3.12a1 3500 (Remove PRECALL opcode)
# Python 3.12a1 3501 (YIELD_VALUE oparg == stack_depth)
# Python 3.12a1 3502 (LOAD_FAST_CHECK, no NULL-check in LOAD_FAST)
# Python 3.12a1 3503 (Shrink LOAD_METHOD cache)

# Python 3.13 will start with 3550

Expand All @@ -420,7 +421,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 = (3502).to_bytes(2, 'little') + b'\r\n'
MAGIC_NUMBER = (3503).to_bytes(2, 'little') + b'\r\n'

_RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little') # For import.c

Expand Down
1 change: 0 additions & 1 deletion Lib/opcode.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,6 @@ def jabs_op(name, op):
"LOAD_METHOD": {
"counter": 1,
"type_version": 2,
"dict_offset": 1,
"keys_version": 2,
"descr": 4,
},
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_dis.py
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@ def test_show_caches(self):
for cache in caches:
self.assertRegex(cache, pattern)
self.assertEqual(caches.count(""), 8)
self.assertEqual(len(caches), 23)
self.assertEqual(len(caches), 22)


class DisWithFileTests(DisTests):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reduce the size of the inline cache for ``LOAD_METHOD`` by 2 bytes.
11 changes: 4 additions & 7 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -4581,12 +4581,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
DEOPT_IF(self_cls->tp_version_tag != read_u32(cache->type_version),
LOAD_METHOD);
/* Treat index as a signed 16 bit value */
int dictoffset = *(int16_t *)&cache->dict_offset;
int dictoffset = self_cls->tp_dictoffset;
Copy link
Member

Choose a reason for hiding this comment

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

I just realised this warns on MSVC. It seems that this dictoffset and the one below needs to be Py_ssize_t

Copy link
Member Author

Choose a reason for hiding this comment

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

assert(dictoffset > 0);
PyDictObject **dictptr = (PyDictObject**)(((char *)self)+dictoffset);
assert(
dictoffset == MANAGED_DICT_OFFSET ||
(dictoffset == self_cls->tp_dictoffset && dictoffset > 0)
);
PyDictObject *dict = *dictptr;
DEOPT_IF(dict == NULL, LOAD_METHOD);
DEOPT_IF(dict->ma_keys->dk_version != read_u32(cache->keys_version),
Expand Down Expand Up @@ -4628,9 +4625,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
_PyLoadMethodCache *cache = (_PyLoadMethodCache *)next_instr;
uint32_t type_version = read_u32(cache->type_version);
DEOPT_IF(self_cls->tp_version_tag != type_version, LOAD_METHOD);
int dictoffset = cache->dict_offset;
int dictoffset = self_cls->tp_dictoffset;
assert(dictoffset > 0);
PyObject *dict = *(PyObject **)((char *)self + dictoffset);
assert(dictoffset == self_cls->tp_dictoffset && dictoffset > 0);
/* This object has a __dict__, just not yet created */
DEOPT_IF(dict != NULL, LOAD_METHOD);
STAT_INC(LOAD_METHOD, hit);
Expand Down
9 changes: 3 additions & 6 deletions Python/specialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@ _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name)
}
}
}
if (dictkind == MANAGED_VALUES || dictkind == MANAGED_DICT || dictkind == OFFSET_DICT) {
if (dictkind == MANAGED_VALUES || dictkind == OFFSET_DICT) {
Py_ssize_t index = _PyDictKeys_StringLookup(keys, name);
if (index != DKIX_EMPTY) {
SPECIALIZATION_FAIL(LOAD_METHOD, SPEC_FAIL_LOAD_METHOD_IS_ATTR);
Expand All @@ -1007,17 +1007,14 @@ _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name)
_Py_SET_OPCODE(*instr, LOAD_METHOD_WITH_VALUES);
break;
case MANAGED_DICT:
*(int16_t *)&cache->dict_offset = (int16_t)MANAGED_DICT_OFFSET;
_Py_SET_OPCODE(*instr, LOAD_METHOD_WITH_DICT);
break;
SPECIALIZATION_FAIL(LOAD_METHOD, SPEC_FAIL_LOAD_METHOD_HAS_MANAGED_DICT);
goto fail;
case OFFSET_DICT:
assert(owner_cls->tp_dictoffset > 0 && owner_cls->tp_dictoffset <= INT16_MAX);
cache->dict_offset = (uint16_t)owner_cls->tp_dictoffset;
_Py_SET_OPCODE(*instr, LOAD_METHOD_WITH_DICT);
break;
case LAZY_DICT:
assert(owner_cls->tp_dictoffset > 0 && owner_cls->tp_dictoffset <= INT16_MAX);
cache->dict_offset = (uint16_t)owner_cls->tp_dictoffset;
_Py_SET_OPCODE(*instr, LOAD_METHOD_LAZY_DICT);
break;
}
Expand Down