perf: cut repeated work on search and cover paths (#1544)

I went through what Shelfmark does per search and per cover and measured
the pieces that showed up. Four of them are cheap to fix, so this is one
PR. Shelfmark runs on a single gevent worker, so any of these blocks
every other request while it runs, and I'm assuming a lot of people run
it on small hardware (a Pi, say).

Cover cache. Every cache hit rewrote the whole `cache_index.json` just
to bump the entry's last-access time. On my box that is 3.7 ms of CPU
per hit at 500 entries and about 100 ms at 15,000 (2.2 MB written each
time), and a results page asks for around 40 covers. The access time now
stays in memory and the index is written at most every 30 seconds, or
whenever something else saves it. A crash inside that window loses some
access times, which only changes eviction order.

Settings reload. `config.refresh()` rebuilt the cache by reading and
parsing a tab's JSON file once per field, so about 250 parses per
refresh, 100 ms of CPU, and the metadata search, providers and config
routes each trigger it. It now reads each file once per rebuild (several
tabs share `settings.json`, so the memo is keyed on the file path).

Library match. When the library check is on, each search result is
compared against every library entry, and the title was split into words
again for every entry. The words are now computed once per result. I
measured 33 ms per result at 5,000 library entries and 106 ms at 20,000,
which is a second or more per page of 40 results. The matching itself is
unchanged.

Cover URL probe. `is_covers_cache_enabled()` creates and deletes a file
in the config directory to check it is writable, and it runs for every
cover, queue task and history row. The answer is remembered for 60
seconds. The probe is 0.045 ms on its own, but it is called a few
hundred times per request in the worst case.

Each change has a test that fails without it (call counts on the index
saves, file opens during a refresh, tokenisation calls, and probe
calls). Ruff, basedpyright on the changed files and the non-e2e suite
pass. The timings are from a dev machine with an empty config, so the
real numbers depend on cache and library size.

shelfmark/config/env.py

@@ -4,6 +4,7 @@ import json
 import os
 import shutil
 import tempfile
+import time
 from pathlib import Path
 
 LOG_LEVELS = ("DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL")
@@ -118,14 +119,36 @@ def _is_config_dir_writable() -> bool:
         return True
 
 
+# The probe above creates and deletes a file, and cover URLs are rewritten per result,
+# per queue task and per history row, so it is remembered for a short time.
+_WRITABLE_PROBE_TTL_SECONDS = 60.0
+_writable_probe: tuple[float, bool] | None = None
+
+
+def _config_dir_writable_cached() -> bool:
+    global _writable_probe
+    now = time.monotonic()
+    if _writable_probe is not None and now - _writable_probe[0] < _WRITABLE_PROBE_TTL_SECONDS:
+        return _writable_probe[1]
+    writable = _is_config_dir_writable()
+    _writable_probe = (now, writable)
+    return writable
+
+
+def reset_config_dir_writable_cache() -> None:
+    """Forget the remembered probe (tests change the directory under it)."""
+    global _writable_probe
+    _writable_probe = None
+
+
 def is_covers_cache_enabled() -> bool:
     """Check if cover caching is enabled (requires setting + writable config dir)."""
     from shelfmark.core.config import config
 
     setting_enabled = config.get("COVERS_CACHE_ENABLED", True)
     if isinstance(setting_enabled, str):
-        return string_to_bool(setting_enabled) and _is_config_dir_writable()
-    return bool(setting_enabled) and _is_config_dir_writable()
+        return string_to_bool(setting_enabled) and _config_dir_writable_cached()
+    return bool(setting_enabled) and _config_dir_writable_cached()
 
 
 # =============================================================================

shelfmark/core/config.py

@@ -124,9 +124,10 @@ class Config:
         self._field_map.clear()
         self._cache.clear()
 
-        for key, (field, tab_name) in registry.get_settings_field_map().items():
-            self._field_map[key] = (field, tab_name)
-            self._cache[key] = registry.get_setting_value(field, tab_name)
+        with registry.config_files_read_once():
+            for key, (field, tab_name) in registry.get_settings_field_map().items():
+                self._field_map[key] = (field, tab_name)
+                self._cache[key] = registry.get_setting_value(field, tab_name)
 
         self._loaded = True
 

shelfmark/core/image_cache.py

@@ -47,6 +47,11 @@ NEGATIVE_CACHE_TTL = 3600
 # Short enough to retry soon, long enough to prevent spam during one page view
 TRANSIENT_CACHE_TTL = 60
 
+# A hit only moves the entry's last-access time, which orders LRU eviction. The whole
+# index is rewritten to disk, so that is done at most this often instead of per hit.
+# A crash inside the window loses some access times, nothing else.
+ACCESS_FLUSH_SECONDS = 30
+
 _MIN_WEBP_HEADER_LENGTH = 12
 HTTP_NOT_FOUND = HTTPStatus.NOT_FOUND
 
@@ -102,6 +107,7 @@ class ImageCacheService:
         self.index_path = cache_dir / "cache_index.json"
         self._lock = threading.RLock()
         self._index: dict[str, dict[str, Any]] = {}
+        self._last_saved = time.monotonic()
 
         # Stats tracking
         self._hits = 0
@@ -195,6 +201,7 @@ class ImageCacheService:
             with temp_path.open("w") as f:
                 json.dump(self._index, f)
             temp_path.rename(self.index_path)
+            self._last_saved = time.monotonic()
         except OSError:
             pass
 
@@ -325,9 +332,11 @@ class ImageCacheService:
                 with image_path.open("rb") as f:
                     data = f.read()
 
-                # Update accessed time
+                # Update accessed time. Written out with the next save, or here once
+                # enough time has passed, not once per hit.
                 entry["accessed_at"] = time.time()
-                self._save_index()
+                if time.monotonic() - self._last_saved >= ACCESS_FLUSH_SECONDS:
+                    self._save_index()
                 result = data, content_type
 
             except OSError:

shelfmark/core/library_index.py

@@ -25,7 +25,8 @@ from shelfmark.core.text_match import (
     author_surname,
     extra_work_tokens,
     isbn_variants,
-    title_tokens_match,
+    significant_tokens,
+    significant_tokens_match,
 )
 
 if TYPE_CHECKING:
@@ -128,8 +129,9 @@ def match_entries(book: BookMetadata, entries: list[LibraryEntry]) -> str | None
     title = book.search_title or book.title
     surname = author_surname(book.search_author or (book.authors[0] if book.authors else ""))
     collection: str | None = None
+    title_toks = significant_tokens(title)
     for entry in entries:
-        if not title_tokens_match(title, set(entry.tokens)):
+        if not significant_tokens_match(title_toks, entry.tokens):
             continue
         if surname is not None and surname not in entry.tokens:
             continue

shelfmark/core/settings_registry.py

@@ -2,6 +2,8 @@
 
 import json
 import os
+from contextlib import contextmanager
+from contextvars import ContextVar
 from dataclasses import dataclass, field
 from pathlib import Path
 from threading import Lock
@@ -419,8 +421,40 @@ def _ensure_config_dir(tab_name: str) -> None:
     config_path.parent.mkdir(parents=True, exist_ok=True)
 
 
+# Set while the whole settings cache is being rebuilt. Every field of a tab used to
+# re-read and re-parse that tab's file, so a rebuild parsed each file once per field.
+_config_file_memo: ContextVar[dict[str, dict[str, Any]] | None] = ContextVar(
+    "config_file_memo", default=None
+)
+
+
+@contextmanager
+def config_files_read_once() -> Iterator[None]:
+    """Read each tab's config file at most once inside the block.
+
+    Only for a read-only pass such as rebuilding the settings cache; a save inside the
+    block would not be seen by reads after it.
+    """
+    token = _config_file_memo.set({})
+    try:
+        yield
+    finally:
+        _config_file_memo.reset(token)
+
+
 def load_config_file(tab_name: str) -> dict[str, Any]:
     """Load a settings tab config file, returning an empty dict on failure."""
+    memo = _config_file_memo.get()
+    if memo is None:
+        return _read_config_file(tab_name)
+    config_path = _get_config_file_path(tab_name)
+    key = str(config_path)  # several tabs share settings.json
+    if key not in memo:
+        memo[key] = _read_config_file(tab_name)
+    return memo[key]
+
+
+def _read_config_file(tab_name: str) -> dict[str, Any]:
     config_path = _get_config_file_path(tab_name)
 
     if not config_path.exists():

shelfmark/core/text_match.py

@@ -7,6 +7,10 @@ library matches titles, authors and ISBNs the same way.
 from __future__ import annotations
 
 import re
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from collections.abc import Collection
 
 DEFAULT_TITLE_MATCH_THRESHOLD = 0.85
 
@@ -57,7 +61,18 @@ def title_tokens_match(
     threshold: float = DEFAULT_TITLE_MATCH_THRESHOLD,
 ) -> bool:
     """True when enough significant title tokens appear in ``haystack_tokens``."""
-    title_toks = significant_tokens(title)
+    return significant_tokens_match(significant_tokens(title), haystack_tokens, threshold)
+
+
+def significant_tokens_match(
+    title_toks: list[str],
+    haystack_tokens: Collection[str],
+    threshold: float = DEFAULT_TITLE_MATCH_THRESHOLD,
+) -> bool:
+    """``title_tokens_match`` for a title already run through ``significant_tokens``.
+
+    Matching one title against many entries should tokenise it once, not per entry.
+    """
     if not title_toks:
         return False
     present = sum(1 for tok in title_toks if tok in haystack_tokens)

tests/config/test_docker_volumes.py

@@ -104,3 +104,20 @@ def test_save_config_file_returns_false_when_config_dir_is_not_writable(tmp_path
             assert save_config_file("downloads", {"key": "value"}) is False
     finally:
         config_dir.chmod(stat.S_IRWXU)
+
+
+def test_covers_cache_probe_is_remembered_for_a_short_time(monkeypatch):
+    from shelfmark.config import env
+
+    probes: list[int] = []
+    monkeypatch.setattr(env, "_is_config_dir_writable", lambda: (probes.append(1), True)[1])
+    clock = [1000.0]
+    monkeypatch.setattr(env.time, "monotonic", lambda: clock[0])
+
+    for _ in range(5):
+        assert env._config_dir_writable_cached() is True
+    assert probes == [1]
+
+    clock[0] += env._WRITABLE_PROBE_TTL_SECONDS + 1
+    assert env._config_dir_writable_cached() is True
+    assert probes == [1, 1]

tests/conftest.py

@@ -29,6 +29,16 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 import pytest
 
 
+@pytest.fixture(autouse=True)
+def _reset_config_dir_writable_cache():
+    """The writable-config-dir probe is remembered; each test starts without it."""
+    from shelfmark.config.env import reset_config_dir_writable_cache
+
+    reset_config_dir_writable_cache()
+    yield
+    reset_config_dir_writable_cache()
+
+
 @pytest.fixture(autouse=True)
 def _clear_torrent_fetch_cache():
     """Keep the shared torrent fetch cache from leaking between tests."""

tests/core/test_image_cache.py

@@ -84,3 +84,40 @@ def test_cover_cache_key_depends_on_the_url_only() -> None:
     assert key != cover_cache_key("https://example.com/cover.jpg?size=large")
     assert key.isalnum() and key == key.lower()
     assert "/" not in key and "." not in key
+
+
+def test_cache_hits_do_not_rewrite_the_index_each_time(tmp_path, monkeypatch) -> None:
+    """A hit only moves the last-access time, so the index is flushed on a timer."""
+    cache = ImageCacheService(tmp_path)
+    assert cache.put("cover-a", b"\xff\xd8\xff" + b"0" * 64, "image/jpeg")
+    saves: list[int] = []
+    real_save = cache._save_index
+    monkeypatch.setattr(cache, "_save_index", lambda: (saves.append(1), real_save())[1])
+
+    before = cache._index["cover-a"]["accessed_at"]
+    for _ in range(5):
+        assert cache.get("cover-a") is not None
+
+    assert saves == []
+    assert cache._index["cover-a"]["accessed_at"] >= before
+
+
+def test_cache_hit_flushes_the_index_once_the_window_has_passed(tmp_path, monkeypatch) -> None:
+    from shelfmark.core import image_cache
+
+    cache = ImageCacheService(tmp_path)
+    assert cache.put("cover-a", b"\xff\xd8\xff" + b"0" * 64, "image/jpeg")
+    saves: list[int] = []
+    real_save = cache._save_index
+    monkeypatch.setattr(cache, "_save_index", lambda: (saves.append(1), real_save())[1])
+
+    now = cache._last_saved
+    monkeypatch.setattr(
+        image_cache.time, "monotonic", lambda: now + image_cache.ACCESS_FLUSH_SECONDS
+    )
+    assert cache.get("cover-a") is not None
+    assert saves == [1]
+
+    # The flush restarts the window.
+    assert cache.get("cover-a") is not None
+    assert saves == [1]

tests/core/test_library_index.py

@@ -466,3 +466,21 @@ def test_an_exact_shelf_title_outranks_a_collection_holding_the_same_book() -> N
     book = _book(title="Dungeon Crawler Carl", authors=["Matt Dinniman"])
 
     assert library_index.match_entries(book, [collection, single]) == "owned"
+
+
+def test_matching_tokenises_the_title_once_for_all_entries(monkeypatch: pytest.MonkeyPatch) -> None:
+    """The title does not change between entries, so it is split into words once."""
+    from shelfmark.core import text_match
+
+    calls: list[str | None] = []
+    real = text_match.significant_tokens
+
+    def counting(text: str | None) -> list[str]:
+        calls.append(text)
+        return real(text)
+
+    monkeypatch.setattr(text_match, "significant_tokens", counting)
+    entries = [_entry("unrelated", f"word{i}", "someone") for i in range(50)]
+
+    assert library_index.match_entries(_book(), entries) is None
+    assert len(calls) <= 2

tests/core/test_settings_reload.py

@@ -0,0 +1,46 @@
+"""Rebuilding the settings cache parses each tab's config file once."""
+
+from __future__ import annotations
+
+import json
+from pathlib import Path
+
+from shelfmark.core import settings_registry
+from shelfmark.core.config import config as app_config
+
+
+def test_refresh_reads_each_config_file_once(tmp_path, monkeypatch):
+    plugins = tmp_path / "plugins"
+    plugins.mkdir()
+    (tmp_path / "settings.json").write_text(json.dumps({"SEARCH_MODE": "universal"}))
+    (plugins / "downloads.json").write_text(json.dumps({"MAX_CONCURRENT_DOWNLOADS": 3}))
+    (plugins / "hardcover.json").write_text(json.dumps({"HARDCOVER_ENABLED": False}))
+
+    opened: dict[str, int] = {}
+    real_open = Path.open
+
+    def counting_open(self, *args, **kwargs):
+        if self.suffix == ".json" and self.parent in (tmp_path, plugins):
+            opened[self.name] = opened.get(self.name, 0) + 1
+        return real_open(self, *args, **kwargs)
+
+    with monkeypatch.context() as patched:
+        patched.setattr(settings_registry, "_get_config_dir", lambda: tmp_path)
+        patched.setattr(Path, "open", counting_open)
+        app_config.refresh(force=True)
+
+    app_config.refresh(force=True)
+    assert opened, "no config file was read"
+    assert max(opened.values()) == 1, opened
+
+
+def test_config_files_are_read_fresh_outside_the_block(tmp_path, monkeypatch):
+    monkeypatch.setattr(settings_registry, "_get_config_dir", lambda: tmp_path)
+    (tmp_path / "settings.json").write_text(json.dumps({"A": 1}))
+
+    with settings_registry.config_files_read_once():
+        assert settings_registry.load_config_file("general") == {"A": 1}
+        (tmp_path / "settings.json").write_text(json.dumps({"A": 2}))
+        assert settings_registry.load_config_file("general") == {"A": 1}
+
+    assert settings_registry.load_config_file("general") == {"A": 2}

tests/core/test_text_match.py

@@ -75,3 +75,11 @@ def test_title_tokens_match_without_significant_tokens_is_false(title):
 )
 def test_normalize_isbn(value, expected):
     assert text_match.normalize_isbn(value) == expected
+
+
+def test_significant_tokens_match_agrees_with_title_tokens_match():
+    haystack = {"alpha", "beta", "gamma"}
+    for title in ("Alpha Beta", "alpha delta", "the of", ""):
+        assert text_match.significant_tokens_match(
+            text_match.significant_tokens(title), haystack
+        ) is text_match.title_tokens_match(title, haystack)