Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#23623 closed Cleanup/optimization (fixed)

ModelChoiceField generates unused temporary cache

Reported by: Thomas Chaumeny Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords: ModelChoiceField, admin
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Thomas Chaumeny)

There is a huge memory consumption when using some admin pages with big production datasets (see also https://groups.google.com/forum/#!topic/django-users/YYlLWyBH_go).

I looked into the ModelChoiceIterator used to generate the choices for a ModelChoiceField, and noticed that an unused temporary queryset cache was generated due to the use of .all() (there is no variable pointing to the cloned queryset after the iteration).

With the proposed PR, this cache is no longer generated. After applying the patch, I noticed that the maximum memory usage dropped from ~200MB to ~150MB for a model with about 30 000 instances.

Change History (6)

comment:1 Changed 6 years ago by Thomas Chaumeny

Description: modified (diff)

comment:2 Changed 6 years ago by Thomas Chaumeny

Description: modified (diff)

comment:3 Changed 6 years ago by Tim Graham

Have you seen #22841? I think that is related to this.

comment:4 Changed 6 years ago by Thomas Chaumeny

I missed that ticket, thanks. My understanding of #22841 is that it proposes a way to "tell" the ModelChoiceField to reuse a queryset instead of reevaluating it every time it is used.

I think that my PR is related but compatible with that proposed change as it handles the case where we don't want to reuse the queryset. That case is currently handled by iterating on queryset.all() — which creates a cloned unevaluated queryset — to construct the choices. My proposition is to use iterator() instead as this is designed precisely for those cases where you don't want to use cache nor to populate it.

Last edited 6 years ago by Thomas Chaumeny (previous) (diff)

comment:5 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In fa534b92dda0771661a98a1ca302ced264d0a6da:

Fixed #23623 -- Reduced memory consumption when generating ModelChoiceField choices

comment:6 Changed 5 years ago by Simon Charette

It looks like this introduced a regression when passing a queryset using the prefetch_related() which is not compatible with iterator() by design #25496.

I would suggest we conditionally use iterator() if no related data should be fetched instead of simply reverting this optimization. Unfortunately there's no public API to determine whether or not a QuerySet is prefetching data and we would have to rely on the qs._prefetch_related_lookups property.

Note: See TracTickets for help on using tickets.
Back to Top