Description

methane

opened on Oct 10, 2025

Issue body actions

Describe your issue

When compiling dict display with duplicated keys, Cython uses __Pyx_PyDict_NewPresized(8) but it actually have only 1 entry.

d = {
  "foo": 1,
  "foo": 1,
  "foo": 1,
  "foo": 1,
  "foo": 1,
  "foo": 1,
  "foo": 1,
  "foo": 1,
}

  /* "dict_test.pyx":2
 * d = {
 *   "foo": 1,             # <<<<<<<<<<<<<<
 *   "foo": 1,
 *   "foo": 1,
*/
  __pyx_t_2 = __Pyx_PyDict_NewPresized(8); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 2, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_2);
  if (PyDict_SetItem(__pyx_t_2, __pyx_mstate_global->__pyx_n_u_foo, __pyx_mstate_global->__pyx_int_1) < 0) __PYX_ERR(0, 2, __pyx_L1_error)
  if (PyDict_SetItem(__pyx_t_2, __pyx_mstate_global->__pyx_n_u_foo, __pyx_mstate_global->__pyx_int_1) < 0) __PYX_ERR(0, 2, __pyx_L1_error)
...

Additionally, using _PyDict_NewPresized() creates regular dict. There is an optimized for unicode-key-only dict but _PyDict_NewPresized() can not use it.

Since _PyDict_NewPresized() is private API, how about just using PyDict_New()?

Relating to #4635, python/cpython#139772 (comment)

Activity

1. da-woods commented on Oct 11, 2025

   

   da-woods

   on Oct 11, 2025

   Collaborator

   More actions

   There's a few separate things here:

   • Cython doesn't account for duplicate keys and so asks for too big a dictionary in this case. I think this is a very minor issue (in that it'll be fairly rare and the consequences are small)
   • _PyDict_NewPresized() avoids the unicode-key optimisation. That may be an issue we should avoid.
   • It's all internal API anyway. That's a reasonable complaint but it would be nice if there was an efficient way to create a dictionary of known size/key-type, since it's a fairly common thing to do.
2. methane commented on Oct 11, 2025

   

   methane

   on Oct 11, 2025

   ContributorAuthor

   More actions

   • Cython doesn't account for duplicate keys and so asks for too big a dictionary in this case. I think this is a very minor issue (in that it'll be fairly rare and the consequences are small)

   FWIW, CPython doesn't use preallocation unless distict keys count is guaranteed.
   That is one of reason why CPython itself doesn't use _PyDict_NewPresized() at all.
3. scoder commented on Oct 11, 2025

   

   scoder

   on Oct 11, 2025

   Contributor

   More actions

   Cython doesn't account for duplicate keys and so asks for too big a dictionary in this case. I think this is a very minor issue (in that it'll be fairly rare and the consequences are small)

   I concur with David here. I think duplicate keys in literals (!) are a) a very rare thing to expect in real code and b) would only lead to slightly larger dicts than needed. I really doubt that this is an issue in practice.

   Also note that we only call _PyDict_NewPresized() for dict literals with more than 8 keys, so it's even less likely that a serious overallocation happens.

   That written, if the question is whether CPython can remove this non-API function, then my answer is probably yes. I'd then be happy to use a public (simpler) variant of _PyDict_FromItems(), which seems a much better replacement.
4. methane commented on Oct 12, 2025

   

   methane

   on Oct 12, 2025

   ContributorAuthor

   More actions

   That written, if the question is whether CPython can remove this non-API function, then my answer is probably yes. I'd then be happy to use a public (simpler) variant of _PyDict_FromItems(), which seems a much better replacement.

   _PyDict_FromItems() is also private API, same to _PyDict_NewPresized().

Issue actions