Skip to content

gh-141510: Optimize {frozen}dict.fromkeys for frozendict#144915

Open
corona10 wants to merge 8 commits intopython:mainfrom
corona10:gh-141510-_PyDict_FromKeys
Open

gh-141510: Optimize {frozen}dict.fromkeys for frozendict#144915
corona10 wants to merge 8 commits intopython:mainfrom
corona10:gh-141510-_PyDict_FromKeys

Conversation

@corona10
Copy link
Member

@corona10 corona10 commented Feb 17, 2026

@corona10 corona10 requested a review from vstinner February 17, 2026 14:15
@corona10 corona10 disabled auto-merge February 17, 2026 15:01
PyObject *d;
int status;

d = _PyObject_CallNoArgs(cls);
Copy link
Member

@methane methane Feb 17, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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.

@corona10 corona10 requested review from methane and vstinner February 17, 2026 15:19
@corona10
Copy link
Member Author

@vstinner @methane I've simplifed the PR to follow your last reivews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants