Fixed #28312 -- Made ModelChoiceIterator.__len__() more memory-efficient.

Instead of loading all QuerySet results in memory, count the number of
entries. This adds an extra query when list() or tuple() is called on the
choices (because both call __len__() then __iter__()) but uses less
memory since the QuerySet results won't be cached. In most cases, the
choices will only be iterated on, meaning that __len__() won't be called
and only one query will be executed.
This commit is contained in:
François Freitag 2018-04-13 18:15:22 -07:00 committed by Tim Graham
parent 9ec77f3d66
commit 3fca95e1ad
3 changed files with 25 additions and 14 deletions

View File

@ -1134,13 +1134,16 @@ class ModelChoiceIterator:
yield ("", self.field.empty_label) yield ("", self.field.empty_label)
queryset = self.queryset queryset = self.queryset
# Can't use iterator() when queryset uses prefetch_related() # Can't use iterator() when queryset uses prefetch_related()
if not queryset._prefetch_related_lookups and queryset._result_cache is None: if not queryset._prefetch_related_lookups:
queryset = queryset.iterator() queryset = queryset.iterator()
for obj in queryset: for obj in queryset:
yield self.choice(obj) yield self.choice(obj)
def __len__(self): def __len__(self):
return len(self.queryset) + (1 if self.field.empty_label is not None else 0) # count() adds a query but uses less memory since the QuerySet results
# won't be cached. In most cases, the choices will only be iterated on,
# and __len__() won't be called.
return self.queryset.count() + (1 if self.field.empty_label is not None else 0)
def choice(self, obj): def choice(self, obj):
return (self.field.prepare_value(obj), self.field.label_from_instance(obj)) return (self.field.prepare_value(obj), self.field.label_from_instance(obj))

View File

@ -96,6 +96,25 @@ class ModelChoiceFieldTests(TestCase):
(self.c3.pk, 'category Third'), (self.c3.pk, 'category Third'),
]) ])
def test_choices_freshness(self):
f = forms.ModelChoiceField(Category.objects.all())
self.assertEqual(len(f.choices), 4)
self.assertEqual(list(f.choices), [
('', '---------'),
(self.c1.pk, 'Entertainment'),
(self.c2.pk, 'A test'),
(self.c3.pk, 'Third'),
])
c4 = Category.objects.create(name='Fourth', slug='4th', url='4th')
self.assertEqual(len(f.choices), 5)
self.assertEqual(list(f.choices), [
('', '---------'),
(self.c1.pk, 'Entertainment'),
(self.c2.pk, 'A test'),
(self.c3.pk, 'Third'),
(c4.pk, 'Fourth'),
])
def test_deepcopies_widget(self): def test_deepcopies_widget(self):
class ModelChoiceForm(forms.Form): class ModelChoiceForm(forms.Form):
category = forms.ModelChoiceField(Category.objects.all()) category = forms.ModelChoiceField(Category.objects.all())
@ -257,17 +276,6 @@ class ModelChoiceFieldTests(TestCase):
(self.c3.pk, 'Third'), (self.c3.pk, 'Third'),
]) ])
def test_queryset_result_cache_is_reused(self):
f = forms.ModelChoiceField(Category.objects.all())
with self.assertNumQueries(1):
# list() calls __len__() and __iter__(); no duplicate queries.
self.assertEqual(list(f.choices), [
('', '---------'),
(self.c1.pk, 'Entertainment'),
(self.c2.pk, 'A test'),
(self.c3.pk, 'Third'),
])
def test_num_queries(self): def test_num_queries(self):
""" """
Widgets that render multiple subwidgets shouldn't make more than one Widgets that render multiple subwidgets shouldn't make more than one

View File

@ -2345,7 +2345,7 @@ class OtherModelFormTests(TestCase):
return ', '.join(c.name for c in obj.colours.all()) return ', '.join(c.name for c in obj.colours.all())
field = ColorModelChoiceField(ColourfulItem.objects.prefetch_related('colours')) field = ColorModelChoiceField(ColourfulItem.objects.prefetch_related('colours'))
with self.assertNumQueries(2): # would be 3 if prefetch is ignored with self.assertNumQueries(3): # would be 4 if prefetch is ignored
self.assertEqual(tuple(field.choices), ( self.assertEqual(tuple(field.choices), (
('', '---------'), ('', '---------'),
(multicolor_item.pk, 'blue, red'), (multicolor_item.pk, 'blue, red'),