Same behaviour of dict.items and defaultdict.items in DTL

[Klaus Bremer]

Recently I've struggled over the defaultdict-issue mentioned here ​https://docs.djangoproject.com/en/1.9/ref/templates/language/#variables.

In my opinion it may be more intuitive if dictionaries and defaultdicts behave the same (as in plain Python).

[Simon Charette]

Do you have an idea of how this could be solved without breaking backward compatibility?

See #25574 for the discussion that added this mention in the docs.

[Klaus Bremer]

Replying to charettes:

Do you have an idea of how this could be solved without breaking backward compatibility?

See #25574 for the discussion that added this mention in the docs.

Thank you for the fast reply! I've read about #25574 before but don't see the point about a backward incompatibility. Existing templates should work as before and a key 'items' will still mask the object-method. But defaultdicts don't have to get typecasted to dicts to work like dicts in the DTL anymore. Or do I miss something?

[Tim Graham]

I'm concerned the proposed PR with have an adverse effect on performance. Maybe you could see what ​djangobench has to say.

[Klaus Bremer]

Well, the isinstance()-call is about 4 times slower than the direct access (d[bit] alone takes about 98 ns. (on a dict with a single entry)).

%%timeit
if isinstance(d, defaultdict):
    pass
res = d[bit]

1000000 loops, best of 3: 378 ns per loop

Whether this is a real bottleneck depends how often Variable.resolve gets called and - may be more important - other time consuming parts of the application. Will try djangobench the next days. Thank you for the review so far.

[Tim Graham]

In #django-dev, FunkyBob says, "My rule is - never pass defaultdict to a template. In the [fairly simplistic] profiling I've run on templates, isinstance is _the_ biggest cause of time in template rendering."

Feel free to reopen if you come up with an alternate implementation.

[Curtis Maloney]

Further to the above:

• remember this is an isinstance test for every step of every lookup.
• what about when someone subclasses dict and gives it a missing method? Same problem, won't be solved
• precedent : once we special case handling of one type, I worry more will come...

[Klaus Bremer]

Some final thoughts, but at the very beginning: I'm in no way annoyed that the patch doesn't get merged!

As funkybob mentioned, isinstance is called for every step of every lookup. So a template like this will cause 4 additional calls:

{% for k, v in d.items %}{{ k }}:{{ v }}{% endfor %}

I have set up a tiny script (with 1000 times the above snippet) for some simple profiling to get a feeling for the impact:

from django.template.base import Context
from django.template.engine import Engine

s = '{% for k, v in d.items %}{{ k }}:{{ v }}{% endfor %}'
d = {'one': [1]}
n = 1000
s *= n

engine = Engine()
t = engine.from_string(s)
context = Context({'d': d,})
#res = t.render(context)

The last line is commented out to get the overhead for preparing the template and the context. Calling 'python -m cProfile -s cumulativ _profile.py' now gives a fairly large output:

322295 function calls (316792 primitive calls) in 0.420 seconds
...
20469    0.005    0.000    0.005    0.000 {built-in method isinstance}

isinstance() gets called 20469 times. Most of these calls are because of 's *= n'. Uncommenting the last line now renders with the unpatched version:

464331 function calls (455828 primitive calls) in 0.543 seconds
...
50472    0.012    0.000    0.012    0.000 {built-in method isinstance}

shows that isinstance gets called excessively and takes about 7 ms additional runtime.
Rendering with the patched version should produce even 4k more calls:

468330 function calls (459827 primitive calls) in 0.517 seconds
...
54471    0.013    0.000    0.013    0.000 {built-in method isinstance}

This is the case and runs 1 ms longer.

To prepare a context for 4k lookups it may take some additional time for accessing a db and the template may not come from a prepared string but from another io-channel. All in all I think that the impact of the patch is insignificant.

@funkybob: you further mentioned a precedent. Well, that may be a point. IMHO it's quiet ok to expect as least surprise using dicts and defaultdicts as these datatypes are builtins resp. from the standard library. One can not assume that individual modified classes derived from these types will have no unexpected behaviour in the way the DTL-lookup works. But that's just my opinion and there may be other ones.