#32980: Improve performance of related manager attribute access
-------------------------------------+-------------------------------------
Reporter: Keryn | Owner: Keryn Knight
Knight |
Type: | Status: assigned
Cleanup/optimization |
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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...
--
Ticket URL: <https://code.djangoproject.com/ticket/32980>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/052.6b7701ab1ab20a32ff8c4516d28dbbf5%40djangoproject.com.
