diff --git a/docs/development/index.rst b/docs/development/index.rst index 7c3d8040..f409851c 100644 --- a/docs/development/index.rst +++ b/docs/development/index.rst @@ -15,3 +15,4 @@ developer-focussed usage information, is provided in the below docs. /development/releasing /development/api /development/assets + /development/performance diff --git a/docs/development/performance.rst b/docs/development/performance.rst new file mode 100644 index 00000000..c6fd2ab8 --- /dev/null +++ b/docs/development/performance.rst @@ -0,0 +1,29 @@ +Performance testing +=================== + +Patchwork provides the ``generate_perf_data`` management command to create +synthetic projects, submitters, series, patches and replies for local performance +testing. You can use this with a disposable database, configured like the +database backend of the instance you want to measure. You can use the command +like so: + +.. code-block:: shell + + python manage.py migrate + python manage.py loaddata default_states + python manage.py generate_perf_data --projects 12 --patches-per-project 100000 + +The defaults create 1.2 million patches in batches of 1,000, with up to ten +patches per series and 100 shared submitters. Each patch has between 1 and 10 +replies (roughly 6.6 million replies with the defaults), with randomly chosen +submitters and dates after the patch. Reply counts and contents are reproducible +with ``--seed``. Patch dates span a year. All patches contain the same small +diff: this is a query-performance dataset, not a +realistic simulation of database storage requirements or mail parsing. + +Generated projects have names such as ``perf-1``. The command refuses to reuse +existing project names or generated submitter addresses. Use a different prefix +for another dataset, or recreate the disposable database. It never deletes data. +Batches are committed independently, so an interrupted run leaves partial data. +Save hooks and signals are bypassed: events, tags, checks and cover letters are +not generated. diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py index 88707b84..e3636465 100644 --- a/patchwork/api/comment.py +++ b/patchwork/api/comment.py @@ -11,6 +11,7 @@ from rest_framework.serializers import HiddenField from rest_framework.serializers import SerializerMethodField +from patchwork.api import utils from patchwork.api.base import BaseHyperlinkedModelSerializer from patchwork.api.base import NestedHyperlinkedIdentityField from patchwork.api.base import MultipleFieldLookupMixin @@ -124,11 +125,16 @@ def get_object(self): def get_queryset(self): cover_id = self.kwargs['cover_id'] - get_object_or_404(Cover, pk=cover_id) + get_object_or_404(Cover.objects.only('id'), pk=cover_id) - return CoverComment.objects.filter(cover=cover_id).select_related( + queryset = CoverComment.objects.filter(cover=cover_id).select_related( 'submitter' ) + if utils.has_version(self.request, '1.2'): + queryset = queryset.select_related('cover__project').defer( + 'cover__content', 'cover__headers' + ) + return queryset class PatchCommentSerializer(BaseCommentListSerializer): @@ -164,11 +170,16 @@ def get_object(self): def get_queryset(self): patch_id = self.kwargs['patch_id'] - get_object_or_404(Patch, id=patch_id) + get_object_or_404(Patch.objects.only('id'), id=patch_id) - return PatchComment.objects.filter(patch=patch_id).select_related( + queryset = PatchComment.objects.filter(patch=patch_id).select_related( 'submitter' ) + if utils.has_version(self.request, '1.2'): + queryset = queryset.select_related('patch__project').defer( + 'patch__content', 'patch__diff', 'patch__headers' + ) + return queryset class CoverCommentList(CoverCommentMixin, ListAPIView): diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py index 389b34c4..841f6adc 100644 --- a/patchwork/api/embedded.py +++ b/patchwork/api/embedded.py @@ -11,6 +11,8 @@ from collections import OrderedDict +from django.utils.functional import cached_property + from rest_framework.serializers import CharField from rest_framework.serializers import PrimaryKeyRelatedField from rest_framework.serializers import SerializerMethodField @@ -49,8 +51,14 @@ def get_choices(self, cutoff=None): [(item.pk, self.display_value(item)) for item in queryset] ) + @cached_property + def serializer(self): + # Fields are bound to a serializer for one request. Reuse the embedded + # serializer instead of rebuilding its fields for every related object. + return self._Serializer(context=self.context) + def to_representation(self, data): - return self._Serializer(context=self.context).to_representation(data) + return self.serializer.to_representation(data) class MboxMixin(BaseHyperlinkedModelSerializer): diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index 443c3822..a388bb0b 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -20,6 +20,7 @@ from rest_framework import status from patchwork.api.base import BaseHyperlinkedModelSerializer +from patchwork.api.base import LinkHeaderPagination from patchwork.api.base import PatchworkPermission from patchwork.api.embedded import PatchSerializer from patchwork.api.embedded import PersonSerializer @@ -337,11 +338,27 @@ class Meta: extra_kwargs = PatchListSerializer.Meta.extra_kwargs +class PatchListPagination(LinkHeaderPagination): + def paginate_queryset(self, queryset, request, view=None): + # Page over IDs before fetching joined rows. Otherwise a deep offset + # can make MySQL fetch and sort every full patch row in the project. + ids = super().paginate_queryset( + queryset.values_list('pk', flat=True), request, view=view + ) + if ids is None: + return None + patches = { + patch.pk: patch for patch in queryset.filter(pk__in=ids).order_by() + } + return [patches[pk] for pk in ids if pk in patches] + + class PatchList(ListAPIView): """List patches.""" permission_classes = (PatchworkPermission,) serializer_class = PatchListSerializer + pagination_class = PatchListPagination filter_class = filterset_class = PatchFilterSet search_fields = ('name',) ordering_fields = ( diff --git a/patchwork/api/series.py b/patchwork/api/series.py index c9f5f54b..6fe315e6 100644 --- a/patchwork/api/series.py +++ b/patchwork/api/series.py @@ -3,6 +3,7 @@ # # SPDX-License-Identifier: GPL-2.0-or-later +from django.db.models import Prefetch from rest_framework.generics import ListAPIView from rest_framework.generics import RetrieveAPIView from rest_framework.serializers import ( @@ -17,6 +18,7 @@ from patchwork.api.embedded import PatchSerializer from patchwork.api.embedded import PersonSerializer from patchwork.api.embedded import ProjectSerializer +from patchwork.models import Patch from patchwork.models import Series @@ -101,7 +103,12 @@ def get_queryset(self): return ( Series.objects.all() .prefetch_related( - 'patches__project', + Prefetch( + 'patches', + queryset=Patch.objects.defer( + 'content', 'diff', 'headers' + ).prefetch_related('project'), + ), 'cover_letter__project', 'dependencies', 'dependents', diff --git a/patchwork/management/commands/generate_perf_data.py b/patchwork/management/commands/generate_perf_data.py new file mode 100644 index 00000000..555585a6 --- /dev/null +++ b/patchwork/management/commands/generate_perf_data.py @@ -0,0 +1,236 @@ +# Patchwork - automated patch tracking system +# SPDX-License-Identifier: GPL-2.0-or-later + +import datetime +import random +import re + +from django.conf import settings +from django.core.management.base import BaseCommand +from django.core.management.base import CommandError +from django.db import reset_queries +from django.db import transaction + +from patchwork.hasher import hash_diff +from patchwork.models import Patch +from patchwork.models import PatchComment +from patchwork.models import Person +from patchwork.models import Project +from patchwork.models import Series +from patchwork.models import State + + +DIFF = '''diff --git a/example.txt b/example.txt +--- a/example.txt ++++ b/example.txt +@@ -1 +1 @@ +-before ++after +''' + + +class Command(BaseCommand): + help = 'Generate synthetic projects and patches for performance testing' + + def add_arguments(self, parser): + parser.add_argument('--projects', type=int, default=12) + parser.add_argument('--patches-per-project', type=int, default=100000) + parser.add_argument('--batch-size', type=int, default=1000) + parser.add_argument('--max-series-size', type=int, default=10) + parser.add_argument('--open-series-percent', type=int, default=20) + parser.add_argument('--archived-percent', type=int, default=10) + parser.add_argument('--seed', type=int, default=42) + parser.add_argument('--prefix', default='perf') + + def handle(self, *args, **options): + for option in ( + 'projects', + 'patches_per_project', + 'batch_size', + 'max_series_size', + ): + if options[option] < 1: + raise CommandError(f'{option.replace("_", "-")} must be > 0') + if options['max_series_size'] > 32767: + raise CommandError('max-series-size must be <= 32767') + for option in ('open_series_percent', 'archived_percent'): + if not 0 <= options[option] <= 100: + raise CommandError( + f'{option.replace("_", "-")} must be between 0 and 100' + ) + prefix = options['prefix'] + if not re.fullmatch(r'[a-zA-Z0-9_-]{1,64}', prefix): + raise CommandError('prefix must be a slug of 1 to 64 characters') + + names = [f'{prefix}-{i + 1}' for i in range(options['projects'])] + emails = [f'{prefix}-{i + 1}@example.invalid' for i in range(100)] + if Project.objects.filter(linkname__in=names).exists() or ( + Project.objects.filter(name__in=names).exists() + ): + raise CommandError( + 'Generated projects already exist; use a new prefix' + ) + if Person.objects.filter(email__in=emails).exists(): + raise CommandError( + 'Generated people already exist; use a new prefix' + ) + open_state = State.objects.filter(action_required=True).first() + closed_state = State.objects.filter(action_required=False).first() + if not open_state or not closed_state: + raise CommandError( + 'An actionable and a non-actionable state are required. ' + 'Load default_states first.' + ) + + rng = random.Random(options['seed']) # noqa: S311 + Person.objects.bulk_create( + [ + Person(email=email, name=f'Test submitter {i + 1}') + for i, email in enumerate(emails) + ] + ) + # Query IDs explicitly: not every backend returns them on bulk insert. + people = list( + Person.objects.filter(email__in=emails).order_by('email') + ) + total_series = 0 + total_replies = 0 + for name in names: + project = Project.objects.create( + linkname=name, + name=name, + listid=f'{name}.example.invalid', + listemail=f'{name}@example.invalid', + use_tags=False, + ) + count, replies = self.generate_project( + project, people, open_state, closed_state, rng, options + ) + total_series += count + total_replies += replies + self.stdout.write( + self.style.SUCCESS( + f'Created {len(names)} projects, {total_series} series, ' + f'{len(names) * options["patches_per_project"]} patches and ' + f'{total_replies} replies.' + ) + ) + + def generate_project( + self, project, people, open_state, closed_state, rng, options + ): + remaining = options['patches_per_project'] + series_count = 0 + patch_count = 0 + reply_count = 0 + diff_hash = hash_diff(DIFF) + # A fixed anchor makes dates reproducible as well as relationships. + anchor = datetime.datetime(2025, 1, 1) + if settings.USE_TZ: + anchor = anchor.replace(tzinfo=datetime.timezone.utc) + while remaining: + series_batch = [] + patches = [] + while remaining and len(patches) < options['batch_size']: + size = min( + rng.randint(1, options['max_series_size']), + remaining, + options['batch_size'] - len(patches), + ) + series_count += 1 + date = anchor - datetime.timedelta( + seconds=rng.randrange(365 * 24 * 60 * 60) + ) + series = Series( + project=project, + name=f'Synthetic series {series_count}', + date=date, + submitter=rng.choice(people), + total=size, + ) + series_batch.append(series) + is_open = rng.randrange(100) < options['open_series_percent'] + # Guarantee one match for open series, at a varied position. + actionable = rng.randint(1, size) if is_open else None + for number in range(1, size + 1): + archived = rng.randrange(100) < options['archived_percent'] + state = rng.choice((open_state, closed_state)) + if number == actionable: + archived, state = False, open_state + elif not is_open and not archived: + state = closed_state + patch_count += 1 + patches.append( + Patch( + project=project, + series=series, + number=number, + name=f'[PATCH {number}/{size}] {series.name}', + msgid=f'<{project.linkname}-{patch_count}@example.invalid>', + date=date, + submitter=series.submitter, + state=state, + archived=archived, + content='Synthetic performance test patch.\n', + diff=DIFF, + hash=diff_hash, + ) + ) + remaining -= size + + # Bulk inserts intentionally bypass save hooks, tags and events. + with transaction.atomic(): + Series.objects.bulk_create(series_batch) + saved_series = { + series.name: series + for series in Series.objects.filter( + project=project, + name__in=[series.name for series in series_batch], + ) + } + for patch in patches: + patch.series = saved_series[patch.series.name] + Patch.objects.bulk_create(patches) + # As with series, query IDs for backends that don't return + # primary keys after bulk inserts. + patch_ids = dict( + Patch.objects.filter( + project=project, + msgid__in=[patch.msgid for patch in patches], + ).values_list('msgid', 'pk') + ) + replies = [] + for patch in patches: + date = patch.date + for number in range(1, rng.randint(1, 10) + 1): + date += datetime.timedelta( + minutes=rng.randint(1, 24 * 60) + ) + replies.append( + PatchComment( + patch_id=patch_ids[patch.msgid], + msgid=f'= options['batch_size']: + PatchComment.objects.bulk_create(replies) + replies = [] + if replies: + PatchComment.objects.bulk_create(replies) + # Development settings enable DEBUG; don't retain millions of SQL + # queries in memory while generating a large dataset. + reset_queries() + if options['verbosity']: + self.stdout.write( + f'{project.linkname}: {patch_count}/' + f'{options["patches_per_project"]} patches' + ) + return series_count, reply_count diff --git a/patchwork/migrations/0051_patch_api_indexes.py b/patchwork/migrations/0051_patch_api_indexes.py new file mode 100644 index 00000000..14c27884 --- /dev/null +++ b/patchwork/migrations/0051_patch_api_indexes.py @@ -0,0 +1,26 @@ +# Generated by Django 6.1.2 on 2026-10-17 03:08 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("patchwork", "0050_alter_userprofile_items_per_page"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddIndex( + model_name="patch", + index=models.Index( + fields=["project", "id"], name="patch_project_id_idx" + ), + ), + migrations.AddIndex( + model_name="patch", + index=models.Index( + fields=["project", "date", "id"], name="patch_project_date_idx" + ), + ), + ] diff --git a/patchwork/migrations/0052_patch_filter_indexes.py b/patchwork/migrations/0052_patch_filter_indexes.py new file mode 100644 index 00000000..ababc953 --- /dev/null +++ b/patchwork/migrations/0052_patch_filter_indexes.py @@ -0,0 +1,26 @@ +# SPDX-License-Identifier: GPL-2.0-or-later + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ('patchwork', '0051_patch_api_indexes'), + ] + + operations = [ + migrations.AddIndex( + model_name='patch', + index=models.Index( + fields=['project', 'submitter', 'id'], + name='patch_project_submitter_idx', + ), + ), + migrations.AddIndex( + model_name='patch', + index=models.Index( + fields=['project', 'name', 'id'], + name='patch_project_name_idx', + ), + ), + ] diff --git a/patchwork/migrations/0053_series_ordering_indexes.py b/patchwork/migrations/0053_series_ordering_indexes.py new file mode 100644 index 00000000..40ff627d --- /dev/null +++ b/patchwork/migrations/0053_series_ordering_indexes.py @@ -0,0 +1,26 @@ +# SPDX-License-Identifier: GPL-2.0-or-later + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ('patchwork', '0052_patch_filter_indexes'), + ] + + operations = [ + migrations.AddIndex( + model_name='series', + index=models.Index( + fields=['project', 'date', 'id'], + name='series_project_date_idx', + ), + ), + migrations.AddIndex( + model_name='series', + index=models.Index( + fields=['project', 'name', 'id'], + name='series_project_name_idx', + ), + ), + ] diff --git a/patchwork/models.py b/patchwork/models.py index 695bfdb0..e9e54307 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -755,6 +755,21 @@ class Meta: ], name='patch_covering_idx_default', ), + # REST lists can order by ID or date without filtering archived. + models.Index( + fields=['project', 'id'], name='patch_project_id_idx' + ), + models.Index( + fields=['project', 'date', 'id'], name='patch_project_date_idx' + ), + models.Index( + fields=['project', 'submitter', 'id'], + name='patch_project_submitter_idx', + ), + # Also cover substring-search counts without fetching patch rows. + models.Index( + fields=['project', 'name', 'id'], name='patch_project_name_idx' + ), ] @@ -1006,6 +1021,16 @@ def __str__(self): class Meta: verbose_name_plural = 'Series' + indexes = [ + models.Index( + fields=['project', 'date', 'id'], + name='series_project_date_idx', + ), + models.Index( + fields=['project', 'name', 'id'], + name='series_project_name_idx', + ), + ] class SeriesReference(models.Model): diff --git a/patchwork/tests/unit/api/test_comment.py b/patchwork/tests/unit/api/test_comment.py index 1cfbcef4..ab8a25c5 100644 --- a/patchwork/tests/unit/api/test_comment.py +++ b/patchwork/tests/unit/api/test_comment.py @@ -67,6 +67,27 @@ def test_list(self): self.assertIn('list_archive_url', resp.data[0]) self.assertIn('addressed', resp.data[0]) + def test_list_query_count(self): + self.project.list_archive_url_format = 'https://example.com/{}' + self.project.save() + comments = [create_cover_comment(cover=self.cover) for _ in range(10)] + for version in ('1.1', '1.2', None): + with self.assertNumQueries(3): + response = self.client.get( + self.api_url(self.cover, version=version) + ) + self.assertEqual(status.HTTP_200_OK, response.status_code) + self.assertEqual(len(comments), len(response.data)) + for comment, data in zip(comments, response.data): + self.assertIn(SAMPLE_CONTENT, data['content']) + if version == '1.1': + self.assertNotIn('list_archive_url', data) + else: + self.assertEqual( + f'https://example.com/{comment.url_msgid}', + data['list_archive_url'], + ) + def test_list_version_1_2(self): """List cover letter comments using API v1.2.""" create_cover_comment(cover=self.cover) @@ -299,6 +320,27 @@ def test_list(self): self.assertIn('list_archive_url', resp.data[0]) self.assertIn('addressed', resp.data[0]) + def test_list_query_count(self): + self.project.list_archive_url_format = 'https://example.com/{}' + self.project.save() + comments = [create_patch_comment(patch=self.patch) for _ in range(10)] + for version in ('1.1', '1.2', None): + with self.assertNumQueries(3): + response = self.client.get( + self.api_url(self.patch, version=version) + ) + self.assertEqual(status.HTTP_200_OK, response.status_code) + self.assertEqual(len(comments), len(response.data)) + for comment, data in zip(comments, response.data): + self.assertIn(SAMPLE_CONTENT, data['content']) + if version == '1.1': + self.assertNotIn('list_archive_url', data) + else: + self.assertEqual( + f'https://example.com/{comment.url_msgid}', + data['list_archive_url'], + ) + def test_list_version_1_2(self): """List patch comments using API v1.2.""" create_patch_comment(patch=self.patch) diff --git a/patchwork/tests/unit/api/test_patch.py b/patchwork/tests/unit/api/test_patch.py index b2890c22..d7ab8113 100644 --- a/patchwork/tests/unit/api/test_patch.py +++ b/patchwork/tests/unit/api/test_patch.py @@ -114,6 +114,31 @@ def test_list_authenticated(self): patch_rsp = resp.data[0] self.assertSerialized(patch, patch_rsp) + def test_list_pagination_order(self): + project = create_project() + for name in ('d', 'a', 'e', 'b', 'c'): + create_patch(project=project, name=name) + create_patch() # Must not leak through the project filter. + for order in ('id', '-id', 'name', '-name'): + expected = list( + Patch.objects.filter(project=project) + .order_by(order) + .values_list('id', flat=True) + )[2:4] + response = self.client.get( + self.api_url(), + { + 'project': project.pk, + 'order': order, + 'per_page': 2, + 'page': 2, + }, + ) + self.assertEqual(status.HTTP_200_OK, response.status_code) + self.assertEqual(expected, [p['id'] for p in response.data]) + self.assertIn('rel="next"', response['Link']) + self.assertIn('rel="prev"', response['Link']) + def test_list_filter_state(self): """Filter patches by state.""" self._create_patch() @@ -238,7 +263,8 @@ def test_list_bug_335(self): series = create_series() create_patches(5, series=series) - with self.assertNumQueries(5): + # Count, page IDs, patch rows and the constant-size prefetches. + with self.assertNumQueries(6): self.client.get(self.api_url()) @utils.store_samples('patch-detail')