What this code in dictobject.c does?

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 :smiley: 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 or MutableMapping. Examples include dict […]

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.)

1 Like

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.