Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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: 1 addition & 0 deletions Include/internal/pycore_dict_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ struct _Py_dict_state {
PyMutex watcher_mutex; // Protects the watchers array (free-threaded builds)
_PyOnceFlag watcher_setup_once; // One-time optimizer watcher setup
PyDict_WatchCallback watchers[DICT_MAX_WATCHERS];
PyObject *empty_frozendict;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Singletons are usually supposed to be stored on _PyRuntime.static_objects.singletons, not on the interpreter state. Is there a reason we're deviating from that convention here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Adding the empty frozendict singleton to _PyRuntime.static_objects.singletons would require to initialize a PyFrozenDictObject structure statically which is complicated. PyFrozenDictObject inherits from PyDictObject which has these two members:

  • PyDictKeysObject *ma_keys
  • PyDictValues *ma_values

We should get access to this empty_keys_struct (Py_EMPTY_KEYS) outside dictobject.c.

static PyDictKeysObject empty_keys_struct = {
        _Py_DICT_IMMORTAL_INITIAL_REFCNT, /* dk_refcnt */
        0, /* dk_log2_size */
        3, /* dk_log2_index_bytes */
        DICT_KEYS_UNICODE, /* dk_kind */
#ifdef Py_GIL_DISABLED
        {0}, /* dk_mutex */
#endif
        1, /* dk_version */
        0, /* dk_usable (immutable) */
        0, /* dk_nentries */
        {DKIX_EMPTY, DKIX_EMPTY, DKIX_EMPTY, DKIX_EMPTY,
         DKIX_EMPTY, DKIX_EMPTY, DKIX_EMPTY, DKIX_EMPTY}, /* dk_indices */
};

Pre-computing hash() is also complicated:

    Py_uhash_t hash = 0;
    hash ^= (1) * 1927868237UL;
    hash ^= (hash >> 11) ^ (hash >> 25);
    hash = hash * 69069U + 907133923UL;
    if (hash == (Py_uhash_t)-1) {
        hash = 590923713UL;
    }

Well. Adding _PyDict_Init() and _PyDict_Fini() to allocate the empty frozendict singleton using dictobject.c code was simpler for me.

};

#define _dict_state_INIT \
Expand Down
2 changes: 2 additions & 0 deletions Include/internal/pycore_pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ extern PyStatus _Py_HashRandomization_Init(const PyConfig *);
extern PyStatus _PyGC_Init(PyInterpreterState *interp);
extern PyStatus _PyAtExit_Init(PyInterpreterState *interp);
extern PyStatus _PyDateTime_InitTypes(PyInterpreterState *interp);
extern PyStatus _PyDict_Init(PyInterpreterState *interp);

/* Various internal finalizers */

Expand All @@ -55,6 +56,7 @@ extern void _PyAST_Fini(PyInterpreterState *interp);
extern void _PyAtExit_Fini(PyInterpreterState *interp);
extern void _PyThread_FiniType(PyInterpreterState *interp);
extern void _PyArg_Fini(void);
extern void _PyDict_Fini(PyInterpreterState *interp);
extern void _Py_FinalizeAllocatedBlocks(_PyRuntimeState *);

extern PyStatus _PyGILState_Init(PyInterpreterState *interp);
Expand Down
12 changes: 8 additions & 4 deletions Lib/test/test_capi/test_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -669,10 +669,14 @@ def test_frozendict_new(self):
self.assertIsNot(fd2, fd)
self.assertEqual(fd2, fd)

# PyFrozenDict_New(NULL) creates an empty dictionary
dct = frozendict_new(NULL)
self.assertEqual(dct, frozendict())
self.assertIs(type(dct), frozendict)
# PyFrozenDict_New(NULL) gets the empty frozendict singleton
singleton = frozendict()
self.assertIs(frozendict_new(NULL), singleton)

# PyFrozenDict_New(iterable) gets the empty frozendict singleton
# if iterable is empty
for iterable in ((), [], {}, frozendict(), [x for x in ()]):
self.assertIs(frozendict_new(iterable), singleton)


if __name__ == "__main__":
Expand Down
55 changes: 47 additions & 8 deletions Lib/test/test_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -1864,15 +1864,51 @@ def test_merge(self):
self.assertIs(frozendict() | fd, fd)

fd = FrozenDict(x=1, y=2)
self.assertEqual(fd | frozendict(), fd)
self.assertEqual(fd | {}, fd)
self.assertEqual(frozendict() | fd, fd)
merge = fd | frozendict()
self.assertEqual(type(merge), frozendict)
self.assertEqual(merge, fd)
merged = fd | {}
self.assertEqual(type(merge), frozendict)
self.assertEqual(merge, fd)
merge = frozendict() | fd
self.assertEqual(type(merge), frozendict)
self.assertEqual(merge, fd)

# gh-149676: Test hash(frozendict | frozendict)
a = frozendict({"a": 1})
b = frozendict({"b": 2})
self.assertEqual(hash(a | b), hash(frozendict({"a": 1, "b": 2})))

@support.cpython_only
def test_empty_singleton(self):
singleton = frozendict()
self.assertFalse(gc.is_tracked(singleton))
self.assertTrue(sys._is_immortal(singleton))

# test constructor
self.assertIs(frozendict(), singleton)
self.assertIs(frozendict(singleton), singleton)
self.assertIs(frozendict({}), singleton)
self.assertIs(frozendict([]), singleton)
self.assertIs(frozendict([x for x in ()]), singleton)

# test merge
for dict_type in (dict, frozendict, FrozenDict):
empty = dict_type()
self.assertIs(singleton | empty, singleton)
self.assertIs(FrozenDict() | empty, singleton)

# test .copy()
self.assertIs(singleton.copy(), singleton)

# test .fromkeys()
self.assertIs(frozendict.fromkeys([]), singleton)

# test pickle (protocol 0 and 1 don't support frozendict)
for proto in range(2, pickle.HIGHEST_PROTOCOL + 1):
fd = pickle.loads(pickle.dumps(singleton, proto))
self.assertIs(fd, singleton)

def test_update(self):
# test "a |= b" operator
d = frozendict(x=1)
Expand Down Expand Up @@ -1935,6 +1971,8 @@ def test_hash_cpython(self):
def test_fromkeys(self):
self.assertEqual(frozendict.fromkeys('abc'),
frozendict(a=None, b=None, c=None))
self.assertEqual(frozendict.fromkeys([]),
frozendict())

# Subclass which overrides the constructor
created = frozendict(x=1)
Expand Down Expand Up @@ -1970,12 +2008,13 @@ def __new__(cls, *args, **kwargs):
self.assertEqual(created, frozendict(x=1))

# Subclass which doesn't override the constructor
class FrozenDictSubclass2(frozendict):
pass

fd = FrozenDictSubclass2.fromkeys("abc")
fd = FrozenDict.fromkeys("abc")
self.assertEqual(fd, frozendict(a=None, b=None, c=None))
self.assertEqual(type(fd), FrozenDictSubclass2)
self.assertEqual(type(fd), FrozenDict)

fd = FrozenDict.fromkeys("")
self.assertEqual(fd, frozendict())
self.assertEqual(type(fd), FrozenDict)

def test_pickle(self):
for proto in range(pickle.HIGHEST_PROTOCOL + 1):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add an empty :class:`frozendict` singleton. Patch by Victor Stinner.
Loading
Loading