Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#23623 closed Cleanup/optimization (fixed)

ModelChoiceField generates unused temporary cache

Reported by: Thomas C Owned by: nobody
Component: Forms Version: dev
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 C)

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 by Thomas C, 10 years ago

Description: modified (diff)

comment:2 by Thomas C, 10 years ago

Description: modified (diff)

comment:3 by Tim Graham, 10 years ago

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

comment:4 by Thomas C, 10 years ago

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 10 years ago by Thomas C (previous) (diff)

comment:5 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In fa534b92dda0771661a98a1ca302ced264d0a6da:

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

comment:6 by Simon Charette, 9 years ago

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