Uh oh!
There was an error while loading. Please reload this page.
gh-141510, PEP 814: Add built-in frozendict type - #144757
Conversation
Add TYPE_FROZENDICT to the marshal module. Add C API functions: * PyAnyDict_Check() * PyAnyDict_CheckExact() * PyFrozenDict_Check() * PyFrozenDict_CheckExact() * PyFrozenDict_New() Add PyFrozenDict_Type C type.
vstinner
commented
Feb 13, 2026
cc @corona10 |
Fix also indentation.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
| } | ||
| if (PyDict_CheckExact(d)) { | ||
| if (PyAnyDict_CheckExact(d)) { |
There was a problem hiding this comment.
Maybe same opinion with @kumaraditya303 we can avoid a lot of critical section if dict is frozen dict.
There was a problem hiding this comment.
I agree, but I would prefer to make such "optimization" change in a separated PR.
| dict_length(PyObject *self) | ||
| { | ||
| return FT_ATOMIC_LOAD_SSIZE_RELAXED(((PyDictObject *)self)->ma_used); | ||
| return GET_USED(_PyAnyDict_CAST(self)); |
There was a problem hiding this comment.
Do we have to use atomic operation for frozendict?
There was a problem hiding this comment.
No, we can skip the atomic operation for frozendict.
There was a problem hiding this comment.
It uses relaxed atomic so it compiles to simple loads so avoiding it has no real benefit.
| }; | ||
| static PyMappingMethods frozendict_as_mapping = { | ||
| .mp_length = dict_length, |
There was a problem hiding this comment.
I think that we can make frozendict_lenth
There was a problem hiding this comment.
I propose to make such optimization in a separated PR.
| static PyMappingMethods frozendict_as_mapping = { | ||
| .mp_length = dict_length, | ||
| .mp_subscript = dict_subscript, |
There was a problem hiding this comment.
Still perfer to make frozendict_subscript, but let's do it at the separate PR.
corona10
left a comment
There was a problem hiding this comment.
We should notice that frozendict is not subclass of dict to follow SC feedback
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Document frozendict after lazy import.
| mp->ma_values == NULL && | ||
| (mp->ma_used >= (mp->ma_keys->dk_nentries * 2) / 3)) | ||
| (mp->ma_used >= (mp->ma_keys->dk_nentries * 2) / 3) && | ||
| !frozendict) |
There was a problem hiding this comment.
We might take this fast-path for frozendict as well. It's just a missed optimization opportunity.
@corona10: Add it to the optimization TODO list :-)
| if (items == NULL) { | ||
| return -1; | ||
| } | ||
| PyObject *frozenset = PyFrozenSet_New(items); |
There was a problem hiding this comment.
The current implementation creates an actual frozenset object which emits surprising error messages:
>>> hash(frozendict(x=[]))
Traceback (most recent call last):
File "<python-input-0>", line 1, in <module>
hash(frozendict(x=[]))
~~~~^^^^^^^^^^^^^^^^^^
TypeError: cannot use 'tuple' as a set element (unhashable type: 'list')
We may change the implementation later to not create an actual frozenset, but reuse frozenset hash code instead, to avoid the surprising errors.
vstinner
commented
Feb 13, 2026
Ah right, done. |
bedevere-bot
commented
Feb 14, 2026
🤖 New build scheduled with the buildbot fleet by @corona10 for commit 9101b1a 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F144757%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
corona10
commented
Feb 14, 2026
Let's wait build bot to pass the refleak test :) |
benediktjohannes
left a comment
There was a problem hiding this comment.
Nitpick: I know this does not primarily belong to the PR, but it's such a small change that a new PR wouldn't make sense for these small "typos".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Adam Johnson <me@adamj.eu>
vstinner
commented
Feb 16, 2026
Tests passed successfully on Refleaks buildbots: |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Add also a title in Doc/library/stdtypes.rst.
vstinner
commented
Feb 16, 2026
@hugovk: I modified a few more titles in the C API doc and the Python doc to use sentence case for headers. |
corona10
left a comment
There was a problem hiding this comment.
lgtm with current implementations , I will soon create PRs for removing unneeded overhead from my side.
sergey-miryanov
left a comment
There was a problem hiding this comment.
Couple of nitpicks.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Benedikt Johannes <benedikt.johannes.hofer@gmail.com>
vstinner
commented
Feb 17, 2026
I merged my PR. Thank you very much for all your reviews! |
Add TYPE_FROZENDICT to the marshal module.
Add C API functions:
Add PyFrozenDict_Type C type.
📚 Documentation preview 📚: https://cpython-previews--144757.org.readthedocs.build/en/144757/c-api/dict.html#frozen-dictionary-objects