diff --git a/codex/views/browser/browser.py b/codex/views/browser/browser.py index 7587972ba..412ba11a9 100644 --- a/codex/views/browser/browser.py +++ b/codex/views/browser/browser.py @@ -199,7 +199,7 @@ def _get_group_and_books(self) -> tuple: num_pages = ceil((group_count + book_count) / BROWSER_MAX_OBJ_PER_PAGE) self.check_page_in_bounds(num_pages) group_qs, book_qs, page_group_count, page_book_count = self.paginate( - group_qs, book_qs, group_count + group_qs, book_qs, group_count, book_count ) # Annotate diff --git a/codex/views/browser/group_mtime.py b/codex/views/browser/group_mtime.py index ecd42067e..2d5279764 100644 --- a/codex/views/browser/group_mtime.py +++ b/codex/views/browser/group_mtime.py @@ -1,7 +1,10 @@ """Group Mtime Function.""" +import hashlib +import json from typing import TYPE_CHECKING +from django.core.cache import cache as _django_cache from django.db.models.aggregates import Aggregate, Max from django.db.models.functions import Greatest from django.db.utils import OperationalError @@ -15,6 +18,9 @@ from django.db.models import Q, Value _FTS5_PREFIX = "fts5: " +_PAGE_MTIME_TTL_SECONDS = 5 +_PAGE_MTIME_CACHE_MISS = object() +_PAGE_MTIME_NONE_SENTINEL = "none" class BrowserGroupMtimeView(BrowserFilterView): @@ -70,8 +76,37 @@ def get_max_bookmark_updated_at_aggregate( self._bmua_agg_cache[key] = aggregate return aggregate + def _page_mtime_cache_key(self, model) -> str: + """Stable key scoped to user + filter-affecting params.""" + user_id = self.request.user.pk if self.request.user.is_authenticated else 0 + group = self.kwargs.get("group", "r") + pks = tuple(self.kwargs.get("pks") or (0,)) + page = self.kwargs.get("page", 1) + filter_keys = ("filters", "search", "q", "order_by", "order_reverse") + params_data = {k: self.params.get(k) for k in filter_keys} + params_str = json.dumps(params_data, sort_keys=True, default=str) + params_hash = hashlib.blake2s(params_str.encode(), digest_size=8).hexdigest() + return ( + f"codex:page_mtime:{user_id}:{model.__name__}:" + f"{group}:{pks}:{page}:{params_hash}" + ) + def get_group_mtime(self, model, group=None, pks=None, *, page_mtime=False): - """Get a filtered mtime for browser pages and mtime checker.""" + """ + Get a filtered mtime for browser pages and mtime checker. + + When ``page_mtime`` is true this is called from the browse page + response — the query is a filtered Max aggregate that runs even on + cachalot misses (writes to Comic/Bookmark invalidate it). A short + TTL cache absorbs concurrent recomputes within that window; real + staleness is bounded by TTL + cachalot's signal-driven invalidation. + """ + cache_key = self._page_mtime_cache_key(model) if page_mtime else "" + if cache_key: + cached = _django_cache.get(cache_key, _PAGE_MTIME_CACHE_MISS) + if cached is not _PAGE_MTIME_CACHE_MISS: + return None if cached == _PAGE_MTIME_NONE_SENTINEL else cached + qs = self.get_filtered_queryset( model, group=group, @@ -101,4 +136,8 @@ def get_group_mtime(self, model, group=None, pks=None, *, page_mtime=False): mtime = None if mtime == NotImplemented: mtime = None + + if cache_key: + stored = _PAGE_MTIME_NONE_SENTINEL if mtime is None else mtime + _django_cache.set(cache_key, stored, _PAGE_MTIME_TTL_SECONDS) return mtime diff --git a/codex/views/browser/paginate.py b/codex/views/browser/paginate.py index a1e0628ea..d847ecd34 100644 --- a/codex/views/browser/paginate.py +++ b/codex/views/browser/paginate.py @@ -13,27 +13,52 @@ class BrowserPaginateView(BrowserPageInBoundsView): """Paginate Groups and Books.""" - def _paginate_section(self, qs: QuerySet, page: int) -> QuerySet: - """Paginate a group or Comic section.""" + def _paginate_section( + self, qs: QuerySet, page: int, total_count: int + ) -> tuple[QuerySet, int]: + """ + Paginate a group or Comic section. + + ``total_count`` is the pre-computed section total (already run by + the caller via one grouped COUNT). It's stuffed into + ``Paginator._count`` so Paginator skips its own COUNT query, and + used to derive the page row count arithmetically instead of + issuing another COUNT on the sliced queryset. + """ + if not total_count: + return qs.model.objects.none(), 0 orphans = 0 if self.model_group == "f" or self.params.get("search") else 5 paginator = Paginator(qs, BROWSER_MAX_OBJ_PER_PAGE, orphans=orphans) + # Shadow the @cached_property with the pre-computed total so + # Paginator.num_pages doesn't issue its own COUNT query. + paginator.count = total_count try: paginator_page = paginator.page(page) qs = paginator_page.object_list + count = paginator_page.end_index() - paginator_page.start_index() + 1 except EmptyPage: if self.model_group != "f": model_name = qs.model.__name__ if qs.model else "UnknownGroup" logger.warning(f"No {model_name}s on page {page}") qs = qs.model.objects.none() + count = 0 - return qs + return qs, count - def _paginate_groups(self, group_qs: QuerySet): + def _paginate_groups( + self, group_qs: QuerySet, group_count: int + ) -> tuple[QuerySet, int]: """Paginate the group object list before books.""" page = self.kwargs.get("page", 1) - return self._paginate_section(group_qs, page) + return self._paginate_section(group_qs, page, group_count) - def _paginate_books(self, book_qs, total_group_count, page_group_count) -> QuerySet: + def _paginate_books( + self, + book_qs: QuerySet, + book_count: int, + total_group_count: int, + page_group_count: int, + ) -> tuple[QuerySet, int]: """Paginate the book object list based on how many group/folders are showing.""" group_remainder = total_group_count % BROWSER_MAX_OBJ_PER_PAGE num_books_on_mixed_page = BROWSER_MAX_OBJ_PER_PAGE - group_remainder @@ -41,32 +66,36 @@ def _paginate_books(self, book_qs, total_group_count, page_group_count) -> Query # There are books after the groups on the same page # Add remainder books without the paginator page_book_qs = book_qs[:num_books_on_mixed_page] - else: - # There are books after the groups on a new page - book_offset = 0 if not group_remainder else num_books_on_mixed_page - page_book_qs = book_qs[book_offset:] + page_book_count = min(num_books_on_mixed_page, book_count) + return page_book_qs, page_book_count + + # There are books after the groups on a new page + book_offset = 0 if not group_remainder else num_books_on_mixed_page + page_book_qs = book_qs[book_offset:] - # Which book page are we on after groups? - page = self.kwargs.get("page", 1) - num_group_and_mixed_pages = ceil( - total_group_count / BROWSER_MAX_OBJ_PER_PAGE - ) - book_only_page = page - num_group_and_mixed_pages + # Which book page are we on after groups? + page = self.kwargs.get("page", 1) + num_group_and_mixed_pages = ceil(total_group_count / BROWSER_MAX_OBJ_PER_PAGE) + book_only_page = page - num_group_and_mixed_pages - page_book_qs = self._paginate_section(page_book_qs, book_only_page) - return page_book_qs + remaining_book_count = max(0, book_count - book_offset) + return self._paginate_section(page_book_qs, book_only_page, remaining_book_count) def paginate( - self, group_qs: QuerySet, book_qs: QuerySet, group_count: int + self, + group_qs: QuerySet, + book_qs: QuerySet, + group_count: int, + book_count: int, ) -> tuple[QuerySet, QuerySet, int, int]: """Paginate the queryset into a group and book object lists.""" if self.TARGET == "opds2": - self._opds_number_of_books = book_qs.count() + self._opds_number_of_books = book_count self._opds_number_of_groups = group_count - page_group_qs = self._paginate_groups(group_qs) - page_group_count = page_group_qs.count() - page_book_qs = self._paginate_books(book_qs, group_count, page_group_count) - page_book_count = page_book_qs.count() + page_group_qs, page_group_count = self._paginate_groups(group_qs, group_count) + page_book_qs, page_book_count = self._paginate_books( + book_qs, book_count, group_count, page_group_count + ) return page_group_qs, page_book_qs, page_group_count, page_book_count diff --git a/tasks/browser-views-perf/stage2.json b/tasks/browser-views-perf/stage2.json new file mode 100644 index 000000000..74f290c5c --- /dev/null +++ b/tasks/browser-views-perf/stage2.json @@ -0,0 +1,50 @@ +{ + "series_pk_used": 325, + "flows": [ + { + "name": "flow_a_root_browse", + "description": "Root browse, no filters, no search.", + "url": "/api/v3/r/0/1", + "cold": { + "status_code": 200, + "num_sql_queries": 16, + "time_taken_ms": 135.547 + }, + "warm": { + "status_code": 200, + "num_sql_queries": 0, + "time_taken_ms": 2.0349999999999997 + } + }, + { + "name": "flow_b_filtered_search", + "description": "Root browse with a search term.", + "url": "/api/v3/r/0/1?q=man", + "cold": { + "status_code": 200, + "num_sql_queries": 15, + "time_taken_ms": 130.28500000000003 + }, + "warm": { + "status_code": 200, + "num_sql_queries": 0, + "time_taken_ms": 1.5939999999999999 + } + }, + { + "name": "flow_c_series_metadata", + "description": "Metadata detail for the largest series.", + "url": "/api/v3/s/325/metadata", + "cold": { + "status_code": 200, + "num_sql_queries": 31, + "time_taken_ms": 229.85 + }, + "warm": { + "status_code": 200, + "num_sql_queries": 0, + "time_taken_ms": 1.729 + } + } + ] +} \ No newline at end of file