﻿id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
32980	Improve performance of related manager attribute access	Keryn Knight		"Currently, getting related managers from an instance is slower than I think it ought to be. Some prelude indicating a relatively expected baseline:
{{{
In [1]: from django.contrib.auth.models import User
In [2]: %timeit User.objects
1.26 µs ± 25.2 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [3]: x = User.objects.first()
In [4]: %timeit x.username
34.2 ns ± 0.574 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
In [5]: %timeit x.get_short_name()
110 ns ± 2.02 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
In [7]: %timeit x.pk
201 ns ± 4.68 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
}}}
So far, all making sense mostly, but then:
{{{
In [8]: %timeit x.groups
7.94 µs ± 40.3 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
In [9]: %timeit x.user_permissions
8.16 µs ± 182 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
}}}
Crikey that's a big difference. What's going on here?
{{{
In [10]: x.groups
Out[10]: <django.db.models.fields.related_descriptors.create_forward_many_to_many_manager.<locals>.ManyRelatedManager at 0x10dc58d60>
In [11]: x.groups
Out[11]: <django.db.models.fields.related_descriptors.create_forward_many_to_many_manager.<locals>.ManyRelatedManager at 0x10da8b490>
In [12]: x.groups is x.groups
Out[12]: False
In [13]: x.user_permissions is x.user_permissions
Out[13]: False
}}}
Aha, it's one of those rare points where seeing the `0x` notation in a repr is useful! The ''manager'' is being re-created on ''every'' access, so even though repeated uses might look innocuous because they're ""the same"" attribute (eg: `x.groups.count()` and then `x.groups.filter(...)`), they're much slower to work with.

{{{
In [13]: %%timeit -n1000
    ...: users = User.objects.prefetch_related('groups')
    ...: for user in users:
    ...:     user.groups
    ...:
8.03 ms ± 108 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
}}}

If we instead cache the returned manager (as is done for the `related_manager_cls`) somewhere, we can amortize the cost of creating a per-instance manager on repeated access:
{{{
In [1]: from django.contrib.auth.models import User
In [2]: x = User.objects.first()
In [4]: %timeit x.groups
554 ns ± 9.86 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [5]: %timeit x.user_permissions
534 ns ± 8.09 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [6]: x.groups is x.groups
Out[6]: True
In [7]: x.user_permissions is x.user_permissions
Out[7]: True
In [8]: %%timeit -n1000
   ...: users = User.objects.prefetch_related('groups')
   ...: for user in users:
   ...:     user.groups
   ...:
6.93 ms ± 207 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
}}}
Because `prefetch_one_level` will use `getattr` to get the manager for every instance seen, the cost is already paid by the time the user wants to then ''use'' the fetched data - even though the manager's underlying queryset will just proxy straight to the prefetched objects cache, it is currently paying a high price to do so.

The how and where and the final implementation (if accepted) are still a bit up for grabs; we cannot cache it on the ''descriptor'' as that would outlive the instance and grow unbounded as instances are seen, similarly using a weakref dict on the descriptor would stop the cached manager outliving the instance but because it's weakly held, it disappears immediately, so doesn't effectively work as a cache. 
That effectively leaves ''somewhere'' as ""on the instance"" as either a private item pushed into `instance.__dict__` or re-using the `field_cache` on the instance's `ModelState`. I have a branch which I'll push shortly showing a proof of concept of how it could be done, having taken some vague guesses on bits - I guarantee it's not ''good enough'' right now, but I think it 'could'' be.

I think this generally only applies to reverse many-to-one and many-to-many managers because the one-to-one and forward many-to-one descriptors are already making use of the `field_cache` in their `__get__` implementation. So I've only concerned myself with those where I've noted that's not the case..."	Cleanup/optimization	new	Database layer (models, ORM)	dev	Normal			Ülgen Sarıkavak	Accepted	0	0	0	0	0	0
