gh-141510: Optimize {frozen}dict.fromkeys for frozendict#144915
gh-141510: Optimize {frozen}dict.fromkeys for frozendict#144915corona10 merged 18 commits intopython:mainfrom
Conversation
| PyObject *d; | ||
| int status; | ||
|
|
||
| d = _PyObject_CallNoArgs(cls); |
There was a problem hiding this comment.
If d is a newly created object, whether it's a dict or a frozendict, wouldn't it be unnecessary to lock it?
If we consider the possibility that d might leak to another thread for some reason, wouldn't it be necessary to lock it even in the case of a frozendict?
class D(frozendict):
def __new__(cls):
return frozendict({"a":1, "b":2})
d = D.fromkeys("def")
print(d)
# frozendict({'a': 1, 'b': 2, 'd': None, 'e': None, 'f': None})In this case, d is exact frozendict, but d can be leaked to other thread.
There was a problem hiding this comment.
I think we could remove the lock for d, but since it has been locked before, would it make sense to test this in a separate PR?
There was a problem hiding this comment.
I don't think that other threads can access d during fromkeys() call.
D.fromkeys() calls dict.fromkeys() and dict.__new__(): the return type is dict, not D.
There was a problem hiding this comment.
D.fromkeys()callsdict.fromkeys()anddict.__new__(): the return type isdict, notD.
dict.fromkeys() is classmethod. D.fromkeys() calls dict.fromkeys(D).
Here's an even worse example.
d0 = None
class D(frozendict):
def __new__(cls):
global d0
d0 = frozendict({"a":1, "b":2})
return d0
d = D.fromkeys("def")
print(d0) # frozendict({'a': 1, 'b': 2, 'd': None, 'e': None, 'f': None})
print(d) # frozendict({'a': 1, 'b': 2, 'd': None, 'e': None, 'f': None})In this case d0 is mutated after it is exposed to Python.
It completely betrays the expectations of a frozendict.
There was a problem hiding this comment.
Oh. This is bad :-( I wrote #144944 to raise an exception if this case occurs.
Uh oh!
There was an error while loading. Please reload this page.