[Django] #30842: Prefetch_related spends considerable time constructing querysets.
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: nobody
Aktsipetrov |
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords: prefetch_related
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
As part of the [https://code.djangoproject.com/ticket/26226 bugfix] django
started constructing N+1 querysets during a typical prefetch_related call
while only 2 sql queries are executed.
This adds a noticeable slowdown. Attaching the flamegraph for queryset
fetching 100 objects.
--
Ticket URL: <https://code.djangoproject.com/ticket/30842>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch_related | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "prefetch.svg.gz" added.
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch_related | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> As part of the [https://code.djangoproject.com/ticket/26226 bugfix]
> django started constructing N+1 querysets during a typical
> prefetch_related call while only 2 sql queries are executed.
>
> This adds a noticeable slowdown. Attaching the flamegraph for queryset
> fetching 100 objects.
New description:
As part of the [https://code.djangoproject.com/ticket/26226 bugfix] django
started constructing N+1 querysets during a typical prefetch_related call
while only 2 sql queries are executed.
This adds a noticeable slowdown. Attaching the flamegraph for queryset
fetching 100 objects, roughly 2/3 of the time are spent there.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30842#comment:1>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch_related | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Alex Aktsipetrov):
Unfortunately I wasn't able to produce a patch for that yet. Experimented
a bit with making queryset construction lazy, but that seems excessively
major for such an issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/30842#comment:2>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Thanks for the report Alex, this was a concern raised during the
implementation https://code.djangoproject.com/ticket/26226#comment:2.
I'm tentatively accepting as we should definitely address this if
possible.
Ideally only proxies to the original queryset would be created to defer
the creation of querysets to only if needed. FWIW
[https://github.com/django/django/pull/6159/commits/c92123cc1dceeb800b3b8900e2e530ed19d78863
#diff-5b0dda5eb9a242c15879dc9cd2121379L1625 N querysets were created
prefetching even before] c92123cc1dceeb800b3b8900e2e530ed19d78863. It's
true that the latter made the matter worse though by performing an
addition `filter` call.
I wonder if performing some form of local memoization per related manager
class to call `manager._apply_rel_filters` only once manager type and
using queryset cloning could speed up things a bit here. Happy to give it
a broad try if that can get you started Alex.
--
Ticket URL: <https://code.djangoproject.com/ticket/30842#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch_related | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/30842#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch_related | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30842#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch_related | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Alex Aktsipetrov):
Replying to [comment:3 Simon Charette]:
> Ideally only proxies to the original queryset would be created to defer
the creation of querysets to only if needed.
I think we can't really defer the creation, since such a proxy would have
to share lots of features with the QuerySet?
Although we probably can create a proxy to defer just filter calls.
But instead I've tried fumbling with QuerySet itself, see the PR. That
seems simpler from implementation perspective and also gives more control
of copying.
> I wonder if performing some form of local memoization per related
manager class to call `manager._apply_rel_filters` only once manager type
and using queryset cloning could speed up things a bit here. Happy to give
it a broad try if that can get you started Alex.
If you think this is a more promising approach, please do.
--
Ticket URL: <https://code.djangoproject.com/ticket/30842#comment:6>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Keywords: prefetch_related | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => duplicate
Comment:
I discovered that #20577 pushes for the same idea of deferring
`_apply_related_filter` application. I think we should close this ticket
as a duplicate and move the discussion there.
--
Ticket URL: <https://code.djangoproject.com/ticket/30842#comment:7>