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.

3 files changed, 25 insertions, 14 deletions

diff --git a/django/forms/models.py b/django/forms/models.py
index d8db4b006a..67006ba390 100644
--- a/django/forms/models.py
+++ b/django/forms/models.py
@@ -1134,13 +1134,16 @@ class ModelChoiceIterator:
             yield ("", self.field.empty_label)
         queryset = self.queryset
         # 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()
         for obj in queryset:
             yield self.choice(obj)
 
     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):
         return (self.field.prepare_value(obj), self.field.label_from_instance(obj))
diff --git a/tests/model_forms/test_modelchoicefield.py b/tests/model_forms/test_modelchoicefield.py
index 733175823f..975873c7f9 100644
--- a/tests/model_forms/test_modelchoicefield.py
+++ b/tests/model_forms/test_modelchoicefield.py
@@ -96,6 +96,25 @@ class ModelChoiceFieldTests(TestCase):
             (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):
         class ModelChoiceForm(forms.Form):
             category = forms.ModelChoiceField(Category.objects.all())
@@ -257,17 +276,6 @@ class ModelChoiceFieldTests(TestCase):
             (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):
         """
         Widgets that render multiple subwidgets shouldn't make more than one
diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py
index 4276bc68f1..1d6d9efa96 100644
--- a/tests/model_forms/tests.py
+++ b/tests/model_forms/tests.py
@@ -2345,7 +2345,7 @@ class OtherModelFormTests(TestCase):
                 return ', '.join(c.name for c in obj.colours.all())
 
         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), (
                 ('', '---------'),
                 (multicolor_item.pk, 'blue, red'),