[GrandComicsDatabase/gcd-django] Fix full-catalog homepage performance (PR #763)
Simone Nardi
Changes
- Guard homepage creator timeline queries with
USE_TEMPLATESADMIN, matching the template’s rendering condition. Preserve the existing selection and aggregation logic when enabled. - Replace the series search’s reverse-relation join with an issue-title subquery, avoiding row multiplication before deduplication.
- Materialize matching series IDs once per request so filtering, pagination and table rendering do not repeat the text search. Preserve substring matching against series names and issue titles.
- Use
select_related()for the foreign-key relationships consumed by the results table. - Handle null and dangling
first_issue/last_issuereferences inSeries.display_publication_dates(), falling back to the existing year-based formatting. This prevents rendering exceptions and the additional queryset evaluation triggered by Django’s debug exception reporter. - Add regression coverage and register the new tests in the existing CI workflow.
Validation
- 186 tests passed on Python 3.13, Django 5.2 and MySQL 8.0.
- Verified search-result equivalence, duplicate elimination, empty queries and case-insensitive matching.
- Verified creator selection with the timeline enabled and absence of creator queries when disabled.
- Verified publication-date formatting with valid, null and missing issue references.
git diff --checkpassed.
No schema migrations, catalog data mutations or infrastructure configuration changes.
Matching IDs are held in memory per request; memory consumption scales with the number of matching series.
You can view, comment on, or merge this pull request online at:
https://github.com/GrandComicsDatabase/gcd-django/pull/763
Commit Summary
- 82e696a Fix full-catalog homepage and series search performance
File Changes
(5 files)
- M .github/workflows/dev-environment-ci.yml (1)
- M apps/gcd/models/series.py (28)
- A apps/gcd/tests/test_front_page_search_performance.py (158)
- M apps/gcd/views/__init__.py (84)
- M apps/gcd/views/search.py (21)
Patch Links:
- https://github.com/GrandComicsDatabase/gcd-django/pull/763.patch
- https://github.com/GrandComicsDatabase/gcd-django/pull/763.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
gemini-code-assist[bot]
@gemini-code-assist[bot] commented on this pull request.
Code Review
This pull request improves performance and robustness for catalog-sized homepage and series searches. It handles missing issue pointers gracefully when retrieving publication dates, conditionally disables the homepage timeline query when managed content is disabled, and optimizes series searches by querying issue titles separately to avoid expensive joins. A review comment suggests further optimizing the series search query in MySQL by fetching matching IDs for series names and issue titles separately in Python and combining them via a set union, avoiding a slow OR subquery.
> + # Search issue titles separately: joining every issue to its series
+ # multiplies rows before DISTINCT and sorting on full catalog dumps.
+ matching_series = Issue.objects.filter(
+ title__icontains=series_name).order_by().values('series_id')
+ q_obj = Q(name__icontains=series_name) | Q(pk__in=matching_series)
+ things = Series.objects.exclude(deleted=True)
+ if series_name:
+ # Filters, pagination and table rendering evaluate separate querysets.
+ # Resolve the text search once per request, then reuse its IDs.
+ matching_ids = list(things.filter(q_obj).order_by()
+ .values_list('pk', flat=True))
+ things = things.filter(pk__in=matching_ids)
The current implementation uses an OR condition combined with an IN (subquery) lookup (Q(name__icontains=series_name) | Q(pk__in=matching_series)). In MySQL, queries with OR and subqueries are notoriously difficult for the optimizer to execute efficiently, often resulting in full table scans or slow dependent subquery evaluations on large catalogs.
To significantly improve performance, we can fetch the matching IDs for the series name and the issue titles separately in Python, and then combine them using a set union. This avoids the complex OR subquery entirely and allows MySQL to execute two highly optimized, simple queries.
- # Search issue titles separately: joining every issue to its series
- # multiplies rows before DISTINCT and sorting on full catalog dumps.
- matching_series = Issue.objects.filter(
- title__icontains=series_name).order_by().values('series_id')
- q_obj = Q(name__icontains=series_name) | Q(pk__in=matching_series)
- things = Series.objects.exclude(deleted=True)
- if series_name:
- # Filters, pagination and table rendering evaluate separate querysets.
- # Resolve the text search once per request, then reuse its IDs.
- matching_ids = list(things.filter(q_obj).order_by()
- .values_list('pk', flat=True))
- things = things.filter(pk__in=matching_ids)
+ things = Series.objects.exclude(deleted=True)
+ if series_name:
+ # Resolve the text search once per request by fetching matching IDs separately.
+ # This avoids a slow OR query with a subquery in MySQL.
+ series_ids_by_name = things.filter(name__icontains=series_name).values_list('pk', flat=True)
+ series_ids_by_issue = Issue.objects.filter(title__icontains=series_name).values_list('series_id', flat=True).distinct()
+ matching_ids = set(series_ids_by_name) | set(series_ids_by_issue)
+ things = things.filter(pk__in=matching_ids)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Simone Nardi
@ProfNardi pushed 1 commit.
- 5d7e153 Update apps/gcd/views/search.py
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
JochenGCD
@jochengcd requested changes on this pull request.
> @@ -363,27 +363,43 @@ def _date_uncertain(self, flag):
return ' ?' if flag else ''
def display_publication_dates(self):
+ # Catalog dumps can omit an issue referenced by these cached pointers.
Instead of modifying the display code, we should prevent the wrong links to be created.
I ran set_first_last_issues() on all affected series, so this situation right now does not exists any more.
When an issue gets deleted, we need to check for:
issue.first_issue_series_set.count()
issue.last_issue_series_set.count()
and if True run set_first_last_issues on the series.
In apps/gcd/views/__init__.py:
> - .filter(issue_count__gt=10)\
- .order_by('-birth_date__month',
- '-birth_date__day',
- 'sort_name').select_related('birth_date')
- creators_count = len(creators)
- end_listed = 10
- if creators_count > 9:
- creator_last = creators[9]
- for i in range(end_listed, creators_count):
- if creators[i].birth_date.month != creator_last.birth_date.month or \
- creators[i].birth_date.day != creator_last.birth_date.day:
- end_listed = i
- break
+ creators = []
+ end_listed = 0
+ # The timeline is only rendered when managed content is enabled.
comment should be changed, USE_TEMPLATESADMIN is different to managed content
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Simone Nardi
@ProfNardi pushed 1 commit.
- 40ab956 Fix issue deletion references as requested in review
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Simone Nardi
@ProfNardi commented on this pull request.
> @@ -363,27 +363,43 @@ def _date_uncertain(self, flag):
return ' ?' if flag else ''
def display_publication_dates(self):
+ # Catalog dumps can omit an issue referenced by these cached pointers.
I reverted the changes to display_publication_dates(). Issue deletion now checks first_issue_series_set.count() and last_issue_series_set.count() and calls set_first_last_issues() on the affected series.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Simone Nardi
@ProfNardi commented on this pull request.
In apps/gcd/views/__init__.py:
> - .filter(issue_count__gt=10)\
- .order_by('-birth_date__month',
- '-birth_date__day',
- 'sort_name').select_related('birth_date')
- creators_count = len(creators)
- end_listed = 10
- if creators_count > 9:
- creator_last = creators[9]
- for i in range(end_listed, creators_count):
- if creators[i].birth_date.month != creator_last.birth_date.month or \
- creators[i].birth_date.day != creator_last.birth_date.day:
- end_listed = i
- break
+ creators = []
+ end_listed = 0
+ # The timeline is only rendered when managed content is enabled.
Updated the timeline comment to refer to USE_TEMPLATESADMIN.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Simone Nardi
@ProfNardi pushed 1 commit.
- 9e56287 Align tests with issue deletion review changes
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
JochenGCD
@jochengcd commented on this pull request.
> + if series.pk == self.series_id: + series = self.series
What shall this do ?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Simone Nardi
@ProfNardi commented on this pull request.
Because the queryset returns a separate Series instance.
Reusing self.series ensures its first/last issue references are updated too, because revision statistics save that cached instance again after deletion.
Otherwise, that save restores the stale references. I verified this: removing these lines makes the deletion test fail, with first_issue pointing to the deleted issue again.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
JochenGCD
@jochengcd commented on this pull request.
I see.
Hmm, I try to avoid code for handling preventable errors.
self.series.set_first_last_issues()
would be enough for what we want to handle here, without the for loop.
There can only be more than one due to dangling first/last issue link in case of moved issue, which we should handle when moving series (I am cleaning up the existing dangled links), not having code here in a different logic place ?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Simone Nardi
@ProfNardi pushed 1 commit.
- be8c004 Simplify series endpoint updates after issue deletion
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Simone Nardi
@ProfNardi commented on this pull request.
Updated as suggested: Issue.delete() now calls self.series.set_first_last_issues() directly after deletion, without the loops. Updated the tests, including a check that saving the cached series again preserves the correct references. All seven targeted tests passed on MySQL.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Simone Nardi
@ProfNardi commented on this pull request.
> + if series.pk == self.series_id: + series = self.series
I found a concrete example in my local database: searching for “Nathan Never” crashes while rendering “Nathan Never gigant” (series ID 147088). Its first_issue_id is 2207972, but that issue is missing from my database. The series has 13 active issues, and the first by sort_code is 2207951.
Is this one of the cases covered by your cleanup? I understand your database may already be fixed while my local copy still contains the stale reference.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
JochenGCD
@jochengcd commented on this pull request.
I cleaned up the production db. With the next dump you would be fine locally as well.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
JochenGCD
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.