Repository navigation
LOAD_ATTR cache does not fully replicate PyObject_GetAttr behavior #86432
Description
| BPO | 42266 |
|---|---|
| Nosy | @pmp-p, @1st1, @pablogsal, @Jongy |
| PRs | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2020-11-05.09:23:52.835>
created_at = <Date 2020-11-04.21:56:10.288>
labels = ['interpreter-core', 'type-bug', '3.10']
title = 'LOAD_ATTR cache does not fully replicate PyObject_GetAttr behavior'
updated_at = <Date 2020-11-05.09:23:52.835>
user = 'https://bugs.python.org/KevinModzelewski'bugs.python.org fields:
activity = <Date 2020-11-05.09:23:52.835>
actor = 'pablogsal'
assignee = 'none'
closed = True
closed_date = <Date 2020-11-05.09:23:52.835>
closer = 'pablogsal'
components = ['Interpreter Core']
creation = <Date 2020-11-04.21:56:10.288>
creator = 'Kevin Modzelewski'
dependencies = []
files = ['49568']
hgrepos = []
issue_num = 42266
keywords = ['patch']
message_count = 7.0
messages = ['380370', '380374', '380375', '380376', '380378', '380379', '380399']
nosy_count = 5.0
nosy_names = ['pmpp', 'yselivanov', 'Kevin Modzelewski', 'pablogsal', 'Yonatan Goldschmidt']
pr_nums = ['23157']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue42266'
versions = ['Python 3.10']
Activity
KevinModzelewski commentedon Nov 4, 2020
The problem is that the descriptor-ness of a type-level attribute is only checked at opcache-set time, not at opcache-hit time.
pablogsal commentedon Nov 4, 2020
Good catch, Kevin! Would you like to submit a PR for fixing this?
pablogsal commentedon Nov 4, 2020
Given that having attributes that are classes is quite uncommon, I think we can not optimize of the attribute itself is a class instead of checking for descriptors on every hit, hurting the performance gains
pablogsal commentedon Nov 4, 2020
Yury, any preference here?
pablogsal commentedon Nov 4, 2020
We could also store the tag of the type object if is a descriptor and compare against that on the cache hit to check that our assumptions are valid. The price here would be an extra pointer on the cache per opcode that may not even be used most of the time.
pablogsal commentedon Nov 4, 2020
s/the attribute itself is a class/the attribute itself is in the class/
pablogsal commentedon Nov 5, 2020
New changeset 80449f2 by Pablo Galindo in branch 'master':
bpo-42266: Handle monkey-patching descriptors in LOAD_ATTR cache (GH-23157)
80449f2