REST: extend performance improvements to other parts of the API

We can trivially extend what we've just done to other parts of the API.

I haven't done much by way of benchmark but we're seeing multiple 'x's
pretty much across the board when filtering.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Stephen Finucane <stephen@that.guru>

diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py
index 9e86d47e00e5cd2e54185c961f55d191d20b43bc..c0c27fe8ece4f1411bb31dca742cc2fee3488054 100644 (file)
--- a/patchwork/api/cover.py
+++ b/patchwork/api/cover.py
@@ -101,7 +101,8 @@ class CoverLetterList(ListAPIView):
 
     def get_queryset(self):
         return CoverLetter.objects.all()\
-            .select_related('project', 'submitter', 'series__project')\
+            .prefetch_related('series__project')\
+            .select_related('project', 'submitter', 'series')\
             .defer('content', 'headers')
 
 

diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
index a3b6f03c6ccddf64cdce97fdbca932895a15f63f..f7b6a6f66f2227e8a510911b694aeba27f6bc0ca 100644 (file)
--- a/patchwork/api/filters.py
+++ b/patchwork/api/filters.py
@@ -164,8 +164,8 @@ class TimestampMixin(BaseFilterSet):
 
 class SeriesFilterSet(TimestampMixin, BaseFilterSet):
 
-    submitter = PersonFilter(queryset=Person.objects.all())
-    project = ProjectFilter(queryset=Project.objects.all())
+    submitter = PersonFilter(queryset=Person.objects.all(), distinct=False)
+    project = ProjectFilter(queryset=Project.objects.all(), distinct=False)
 
     class Meta:
         model = Series
@@ -174,12 +174,12 @@ class SeriesFilterSet(TimestampMixin, BaseFilterSet):
 
 class CoverLetterFilterSet(TimestampMixin, BaseFilterSet):
 
-    project = ProjectFilter(queryset=Project.objects.all())
+    project = ProjectFilter(queryset=Project.objects.all(), distinct=False)
     # NOTE(stephenfin): We disable the select-based HTML widgets for these
     # filters as the resulting query is _huge_
     series = BaseFilter(queryset=Project.objects.all(),
-                        widget=MultipleHiddenInput)
-    submitter = PersonFilter(queryset=Person.objects.all())
+                        widget=MultipleHiddenInput, distinct=False)
+    submitter = PersonFilter(queryset=Person.objects.all(), distinct=False)
 
     class Meta:
         model = CoverLetter
@@ -193,10 +193,10 @@ class PatchFilterSet(TimestampMixin, BaseFilterSet):
     # NOTE(stephenfin): We disable the select-based HTML widgets for these
     # filters as the resulting query is _huge_
     series = BaseFilter(queryset=Series.objects.all(),
-                        widget=MultipleHiddenInput)
-    submitter = PersonFilter(queryset=Person.objects.all())
-    delegate = UserFilter(queryset=User.objects.all())
-    state = StateFilter(queryset=State.objects.all())
+                        widget=MultipleHiddenInput, distinct=False)
+    submitter = PersonFilter(queryset=Person.objects.all(), distinct=False)
+    delegate = UserFilter(queryset=User.objects.all(), distinct=False)
+    state = StateFilter(queryset=State.objects.all(), distinct=False)
     hash = CharFilter(lookup_expr='iexact')
 
     class Meta:
@@ -214,7 +214,7 @@ class PatchFilterSet(TimestampMixin, BaseFilterSet):
 
 class CheckFilterSet(TimestampMixin, BaseFilterSet):
 
-    user = UserFilter(queryset=User.objects.all())
+    user = UserFilter(queryset=User.objects.all(), distinct=False)
 
     class Meta:
         model = Check
@@ -227,13 +227,17 @@ class EventFilterSet(TimestampMixin, BaseFilterSet):
     # filters as the resulting query is _huge_
     # TODO(stephenfin): We should really use an AJAX widget of some form here
     project = ProjectFilter(queryset=Project.objects.all(),
-                            widget=MultipleHiddenInput)
+                            widget=MultipleHiddenInput,
+                            distinct=False)
     series = BaseFilter(queryset=Series.objects.all(),
-                        widget=MultipleHiddenInput)
+                        widget=MultipleHiddenInput,
+                        distinct=False)
     patch = BaseFilter(queryset=Patch.objects.all(),
-                       widget=MultipleHiddenInput)
+                       widget=MultipleHiddenInput,
+                       distinct=False)
     cover = BaseFilter(queryset=CoverLetter.objects.all(),
-                       widget=MultipleHiddenInput)
+                       widget=MultipleHiddenInput,
+                       distinct=False)
 
     class Meta:
         model = Event
@@ -245,8 +249,8 @@ class EventFilterSet(TimestampMixin, BaseFilterSet):
 
 class BundleFilterSet(BaseFilterSet):
 
-    project = ProjectFilter(queryset=Project.objects.all())
-    owner = UserFilter(queryset=User.objects.all())
+    project = ProjectFilter(queryset=Project.objects.all(), distinct=False)
+    owner = UserFilter(queryset=User.objects.all(), distinct=False)
 
     class Meta:
         model = Bundle

diff --git a/patchwork/api/series.py b/patchwork/api/series.py
index df28f95dab1be11f92550964d3b582053f706ee5..e4cffaa3b384627855f2e5ab06fe5d2986c5a974 100644 (file)
--- a/patchwork/api/series.py
+++ b/patchwork/api/series.py
@@ -56,8 +56,8 @@ class SeriesMixin(object):
 
     def get_queryset(self):
         return Series.objects.all()\
-            .prefetch_related('patches__project',)\
-            .select_related('submitter', 'cover_letter__project', 'project')
+            .prefetch_related('patches__project', 'cover_letter__project')\
+            .select_related('submitter', 'project')
 
 
 class SeriesList(SeriesMixin, ListAPIView):

diff --git a/patchwork/tests/api/test_cover.py b/patchwork/tests/api/test_cover.py
index 16cc0cd96f26559936ea6ef38445a0e15e07972b..5eeb1902e1d17cc0cc7e2718db6e07e65136d20a 100644 (file)
--- a/patchwork/tests/api/test_cover.py
+++ b/patchwork/tests/api/test_cover.py
@@ -127,7 +127,7 @@ class TestCoverLetterAPI(utils.APITestCase):
         series = create_series()
         create_covers(5, series=series)
 
-        with self.assertNumQueries(2):
+        with self.assertNumQueries(3):
             self.client.get(self.api_url())
 
     @utils.store_samples('cover-detail')

diff --git a/patchwork/tests/api/test_series.py b/patchwork/tests/api/test_series.py
index 87518719b8872c98450204662f25e3d7649d1f72..491dd616e4229e695c190423dd08d8ad46ae9f83 100644 (file)
--- a/patchwork/tests/api/test_series.py
+++ b/patchwork/tests/api/test_series.py
@@ -139,10 +139,17 @@ class TestSeriesAPI(utils.APITestCase):
         self.assertNotIn('web_url', resp.data[0]['patches'][0])
 
     def test_list_bug_335(self):
-        """Ensure we retrieve the embedded cover letter project once."""
-        self._create_series()
-
-        with self.assertNumQueries(4):
+        """Ensure we retrieve the embedded cover letter project in O(1)."""
+        project_obj = create_project(linkname='myproject')
+        person_obj = create_person(email='test@example.com')
+        for i in range(10):
+            series_obj = create_series(
+                project=project_obj, submitter=person_obj,
+            )
+            create_cover(series=series_obj)
+            create_patch(series=series_obj)
+
+        with self.assertNumQueries(6):
             self.client.get(self.api_url())
 
     @utils.store_samples('series-detail')