The code is part of dict_merge()
(link), and it’s this one:
if (PyDict_Check(b) && (Py_TYPE(b)->tp_iter == (getiterfunc)dict_iter))
What Py_TYPE(b)->tp_iter == (getiterfunc)dict_iter
exactly does? Is not PyDict_Check(b)
sufficient?
The code is part of dict_merge()
(link), and it’s this one:
if (PyDict_Check(b) && (Py_TYPE(b)->tp_iter == (getiterfunc)dict_iter))
What Py_TYPE(b)->tp_iter == (getiterfunc)dict_iter
exactly does? Is not PyDict_Check(b)
sufficient?
IIUC, PyDict_Check
will return true for any dict
subclass, including those with a replacement __iter__
method. The check could be if (PyDict_CheckExact(b))
, but that would preclude any other simple dict subclasses, like defaultdict
.
defaultdict
type has no Py_TPFLAGS_DICT_SUBCLASS
, that is checked by PyDict_Check
.
Yes, it does.
Mh. It’s true. But anyway, I do not understand what does tp_iter
have to do with the rest of dict_merge
code.
I mean, if the check is positive, the object is cast to PyDictObject*
struct, and ma_used
, ma_keys
and ma_values
attributes are used. But I don’t think these attributes are touched if you simply change the tp_iter
function from the default one of dict
. Or not?
The check is there so that subclasses that define tp_iter to be different from the regular dict iteration will still do the right thing. If a subclass defines iteration differently, directly accessing the dict data may do the wrong thing. The shortcut the code takes is only valid if the subclass does not change what iteration does.
Ok, I understand… but this way, even a cosmetic change to tp_iter
can slow down a subclass.
Yes. On the other hand, not doing it this way makes the code incorrect. It doesn’t matter how fast incorrect code is.
But can’t be a way to say “okay, I changed tp_iter
, but you can merge with the fast way anyway”? I don’t know, maybe adding a tp_members
.
There could be such a way, but it would come at a cost. You don’t want to optimise for the uncommon cases. Are there specific cases you have in mind where a subclass re-implements iteration but it’s so important that dict_merge is as fast as possible that the re-implementation should be ignored?
Well, yes I’m trying to implement a frozendict
, and probably, since it is immutable, I would create the dictiterobject
object only once, cache it and if it’s requested another time, I’ll return a shallow copy of it, changing only the di_pos
if it’s reversed or not.
You can’t cache iterators. They are not immutable. They represent iteration state.
Yes, that’s why edit: i copy it and I would change di_pos
, and di_pos
only. This is possible, since object that they iterate over is immutable.
Another fact: what about collections.abc.Mapping
? Even if they implement a dict-like
API, they are not subclasses of dict
, and usually they do the trick of having a member that is the "real" dictionary. See for example my implementation of MultiDict.
In that case, not only __iter__
changed, but the “real” dict
is not the object itself, but o._dict
. But dict
could use the fast merging anyway.
You can’t cache iterators. Different iterators should have different state.
I wrote that I copy the cached iterator and I change di_pos
. It’s not the same iterator.
What about abc.Mapping
? I think it’s a big problem, since the current dict_merge
implementation penalize them.
abc.Mapping
is not a subclass of dict
, so it is not relevant.
mapping: A container object that supports arbitrary key lookups and implements the methods specified in
collections.abc.Mapping
orMutableMapping
. Examples includedict
[…]
Source: official Py3 glossary.
Even if it’s not a subclass, if it walks like a duck and quacks like a duck, it’s a duck.
Furthermore I don’t think users are very interested in finesses like “it not a subclass”. An abc.Mapping
and dict
are maps. So, if I have
a1 = {"Marco": "Sulla"}
a2 = MyCustomMapping(a1)
and I perform
{}.update(a1)
{}.update(a2)
I expect that, if MyCustomMapping
uses a dict
or a subclass of dict
as the “real” map, and the iterator is simply something like
def __iter__(self):
return iter(self._dict)
then the two operations should have the same performance. But sadly this is not true.
That’s not a realistic expectation, and I don’t know how you came to it. The abstract baseclass is purely about semantics, not performance characteristics. There are mappings in the standard library that have entirely different performance characteristics, let alone in third-party packages. That’s normal, and expected, and reasonable, and good.
The dict implementation only applies to dicts and dict subclasses. By and large it optimises for regular dicts, because that’s both the most common and the most performance-critical case in the most common Python code. (Do keep in mind that dicts are used throughout Python, so even if you do have a dict subclass in a performance-critical piece of code, speeding up the subclass by slowing down the dict type may not be a win. What could certainly be a performance win would be to not use a dict subclass at all.)
speeding up the subclass by slowing down the dict type may not be a win
The fact is dict
sub-classes that not override __iter__
are not penalized at all in dict_merge()
, if you read the code.
What you said is not correct. dict_merge
already slows down itself to check if the object is a subclass of dict
and tp_iter
is not changed. If you say this is not a win, why it’s in the CPython code?
Furthermore, it’s a very common case to have a Mapping
that uses a dict
in its attributes as the real dict
. And this objects does not slow down itself. it slows down dict
, because it’s dict
that do the slow dict_merge()
method, if the object is not a dict
or a subclass of dict
.
IMHO, Python
should create an abstract type basedict
in pure C, and dict
should be a subclass of it. There are the bases for this: there’s the dict-common.h
header file. This way, if you want to create a new map type, it’s better to use basedict
instead of abc.(Mutable)Mapping
, since it’s more performant.