perf(images): share and warm one listing per output root (#348)

The Images page, its sidebar counts, the subfolder list and the legacy
gallery each rebuilt the output listing when the 30 s cache expired, all
at once, and the subfolder list computed relative paths on the event
loop. Thumbnail writes also threw the listing away.

Keep one RootListing per output root (py/api/gallery_listing.py):
- one walk per root at a time; concurrent readers wait for it (shielded)
- a listing older than 30 s is served at once and refreshed in the
  background
- the walk, the comparison with the previous listing and both folder
  sets run in the executor; an unchanged walk keeps the same list object
- os.scandir probe: a missing root fails the walk and keeps the previous
  listing (one warning per failure streak)
- Rescan waits for a walk started after the click and is a 500 when it
  fails, even with a previous listing
- deletes keep the blocking invalidation; a generation counter that
  survives drops discards walks that raced it
- thumbnail jobs, conversion and the legacy generate/clear routes no
  longer drop the listing
- warm-up on the host's on_startup, cancelled on on_cleanup

Adds tools/vendor/perf/images_load_probe.mjs (maintainer tool, not CI)
and the TESTING.md section "Images listing cache (#348)".

py/api/__init__.py

@@ -47,6 +47,7 @@ from .review_events import ReviewEventsMixin
 from .saved_views import SavedViewRoutesMixin
 from .history import HistoryRoutesMixin
 from .images import ImageRoutesMixin
+from .gallery_listing import WALK_STOPPED, RootListing, WalkOutcome
 from .admin import AdminRoutesMixin
 from .settings_routes import SettingsRoutesMixin
 from .job_routes import JobRoutesMixin
@@ -344,8 +345,10 @@ class PromptManagerAPI(
         self._captures_closed = False
         self._cached_output_dir = None  # Lazy-cached by _find_comfyui_output_dir()
         self._html_cache = {}  # Cached HTML file contents keyed by path
-        self._gallery_cache = {}  # dict: path_str -> (files, timestamp)
-        self._gallery_cache_ttl = 30  # Cache TTL in seconds
+        # One shared listing per output root, keyed by str(root), and the
+        # background walks refreshing them (py/api/gallery_listing.py, #348).
+        self._root_listings: dict[str, RootListing] = {}
+        self._listing_tasks: set = set()
         # Workspace listing values and sort orders derived from a cached scan
         # (py/api/images.py _workspace_listed/_workspace_orders).
         self._workspace_listed_cache = {}
@@ -465,20 +468,54 @@ class PromptManagerAPI(
             return await loop.run_in_executor(None, call)
         return await loop.run_in_executor(None, func, *args)
 
-    def invalidate_gallery_cache(self):
-        """Invalidate the cached gallery file listing.
+    def _root_listing(self, root) -> RootListing:
+        """The shared listing record of one output root (created empty)."""
+        return self._root_listings.setdefault(str(root), RootListing())
 
-        Called by the image monitor when new files are detected. The workspace
-        keeps its per-file values and card facts, so the next request rebuilds
-        only new or changed files (see ImageRoutesMixin._workspace_listed and
-        _card_facts); its sort orders
-        are dropped, as the rescan replaces them anyway, and so is the
-        favorites lookup, which holds the old scan list.
+    def invalidate_gallery_cache(self):
+        """Drop every root's listing, so the next read waits for a new walk.
+
+        Called after files are deleted through the app (duplicate cleanup and
+        the legacy duplicate delete), never for thumbnail writes. A walk
+        running now is not stored (its generation moved on). The workspace
+        keeps its per-file values and card facts, so the next request
+        rebuilds only new or changed files (see ImageRoutesMixin.
+        _workspace_listed and _card_facts); its sort orders are dropped, as
+        the walk replaces them anyway, and so is the favorites lookup, which
+        holds the old list.
         """
-        self._gallery_cache = {}
+        for record in self._root_listings.values():
+            record.drop()
         self._workspace_order_cache = {}
         self._favorite_listing_cache = {}
 
+    async def warm_gallery_listings(self, app=None):
+        """Start one background walk per output root (aiohttp on_startup).
+
+        Registered by the root ``__init__.py``, so the first Images open after
+        ComfyUI starts finds the listing built. It never fails startup: a
+        root that cannot be walked is retried by the first read.
+        """
+        try:
+            for root in self._get_all_output_dirs():
+                self._schedule_walk(root)
+        except Exception as e:
+            self.logger.warning(f"Could not warm the Images listing: {e}")
+
+    async def cleanup_gallery_listings(self, app=None):
+        """Stop the background listing walks (aiohttp on_cleanup)."""
+        tasks = list(self._listing_tasks)
+        for task in tasks:
+            task.cancel()
+        if tasks:
+            await asyncio.gather(*tasks, return_exceptions=True)
+        self._listing_tasks.clear()
+        # A walk cancelled before its first step never ran its own cleanup.
+        for record in self._root_listings.values():
+            flight, record.flight = record.flight, None
+            if flight is not None and not flight.done():
+                flight.set_result(WalkOutcome.failed(RuntimeError(WALK_STOPPED)))
+
     def reset_gallery_roots_cache(self):
         """Drop every cached listing and derived workspace value (roots changed)."""
         self.invalidate_gallery_cache()

py/api/gallery_listing.py

@@ -0,0 +1,129 @@
+"""One shared listing per output root, and the pure helpers behind it (#348).
+
+The API keeps one ``RootListing`` per output root (``_root_listings`` in
+``py/api/__init__.py``). The record is read and written only on the event
+loop; the walk itself, the comparison with the previous listing and both
+folder sets run in the executor through ``walk_root``. Routes live in
+``py/api/images.py`` (``_listing``); this module has no aiohttp import so
+tests can exercise it without a server.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import os
+from dataclasses import dataclass
+from pathlib import PurePath
+from typing import Callable, NamedTuple
+
+# A listing older than this (seconds) is served as it is, and one background
+# walk replaces it. Read as ``gallery_listing.REFRESH_AFTER`` so tests can
+# patch it.
+REFRESH_AFTER = 30
+
+# A reader with no listing gives up (500) after this many walks in a row were
+# discarded by invalidations made while they ran.
+MAX_DISCARDED_WALKS = 3
+
+WALK_STOPPED = "The listing walk was stopped"
+
+STORED = "stored"
+DISCARDED = "discarded"
+FAILED = "failed"
+
+
+class WalkOutcome(NamedTuple):
+    """What one walk ended with; a walk's flight future resolves to this."""
+
+    kind: str
+    error: BaseException | None = None
+
+    @classmethod
+    def stored(cls) -> "WalkOutcome":
+        return cls(STORED)
+
+    @classmethod
+    def discarded(cls) -> "WalkOutcome":
+        return cls(DISCARDED)
+
+    @classmethod
+    def failed(cls, error: BaseException) -> "WalkOutcome":
+        return cls(FAILED, error)
+
+
+@dataclass
+class RootListing:
+    """The shared listing of one output root.
+
+    A record is never deleted: ``drop()`` clears the listing and bumps
+    ``generation``, so a walk that started before a drop can never be stored
+    after it.
+    """
+
+    # (path, mtime) newest first, as _scan_gallery_files_sync returns it.
+    files: list | None = None
+    folders: frozenset | None = None
+    folders_with_ancestors: frozenset | None = None
+    # Monotonic time the last walk finished, stored or failed.
+    walked_at: float | None = None
+    # Number of the last walk started; a running walk is always that one.
+    walk_seq: int = 0
+    # Number of the walk whose files are stored (Rescan waits on it).
+    stored_seq: int = 0
+    generation: int = 0
+    # The running walk's future (resolves to a WalkOutcome), or None.
+    flight: asyncio.Future | None = None
+    # True while walks keep failing: one warning per streak.
+    failing: bool = False
+
+    def drop(self) -> None:
+        self.files = None
+        self.folders = None
+        self.folders_with_ancestors = None
+        self.walked_at = None
+        self.generation += 1
+
+
+def folder_sets(root, files) -> tuple[frozenset, frozenset]:
+    """``(folders, folders_with_ancestors)`` of the files listed under *root*.
+
+    Folders keep the spelling ``/gallery/subfolders`` has always answered:
+    ``str(relative.parent)`` (native separators) for the folders holding
+    files, plus every ancestor joined by ``/``.
+    """
+    folders = set()
+    for path, _mtime in files:
+        rel_dir = str(path.relative_to(root).parent)
+        if rel_dir and rel_dir != ".":
+            folders.add(rel_dir)
+    with_ancestors = set(folders)
+    for folder in folders:
+        parts = folder.replace("\\", "/").split("/")
+        for i in range(1, len(parts)):
+            with_ancestors.add("/".join(parts[:i]))
+    return frozenset(folders), frozenset(with_ancestors)
+
+
+def same_listing(old, new) -> bool:
+    """True when two listings hold the same ``(path, mtime)`` pairs in order."""
+    if old is None or new is None:
+        return False
+    return old is new or old == new
+
+
+def walk_root(
+    scan_fn: Callable[[PurePath], list], root, previous_files
+) -> tuple[list, frozenset, frozenset]:
+    """The single executor job of a walk: ``(files, folders, with_ancestors)``.
+
+    Opens the root first, so a missing or unreadable root raises ``OSError``
+    instead of listing nothing. Returns *previous_files* itself when nothing
+    changed, which keeps the identity-keyed caches built on it warm.
+    """
+    with os.scandir(root):
+        pass
+    files = scan_fn(root)
+    if same_listing(previous_files, files):
+        files = previous_files
+    folders, with_ancestors = folder_sets(root, files)
+    return files, folders, with_ancestors

py/api/images.py

@@ -22,6 +22,15 @@ from aiohttp import web
 from PIL import Image, UnidentifiedImageError
 
 from .jobs import MAX_OUTCOME_ROWS, THUMBNAILS_JOB_KIND
+from . import gallery_listing
+from .gallery_listing import (
+    DISCARDED,
+    FAILED,
+    MAX_DISCARDED_WALKS,
+    WALK_STOPPED,
+    WalkOutcome,
+    walk_root,
+)
 from . import thumbnail_settings
 from .thumbnail_encode import (
     PREVIEW_HELD,
@@ -1296,16 +1305,107 @@ class ImageRoutesMixin:
         return found
 
     async def _get_gallery_files(self, output_path):
-        """Get gallery files with per-directory TTL cache."""
-        now = _time.monotonic()
-        key = str(output_path)
-        cached = self._gallery_cache.get(key)
-        if cached and (now - cached[1]) < self._gallery_cache_ttl:
-            return cached[0]
+        """The shared listing of one output root: ``[(path, mtime)]``, newest first."""
+        return (await self._listing(output_path)).files
 
-        files = await self._run_in_executor(self._scan_gallery_files_sync, output_path)
-        self._gallery_cache[key] = (files, now)
-        return files
+    async def _listing(self, output_path, *, fresh=False):
+        """The shared ``RootListing`` of *output_path*, with a listing (#348).
+
+        A listing that exists is returned at once; when it is older than
+        ``REFRESH_AFTER`` one background walk replaces it. With no listing,
+        the caller waits for the root's single walk. ``fresh`` (Rescan) waits
+        for a walk that started after this call, and raises when it fails
+        even if a previous listing exists.
+        """
+        record = self._root_listing(output_path)
+        needed = record.walk_seq + 1 if fresh else 0
+        discarded = 0
+        while True:
+            if record.files is not None and record.stored_seq >= needed:
+                if not fresh:
+                    self._refresh_if_stale(output_path, record)
+                return record
+            flight = record.flight
+            if flight is None:
+                flight = self._start_walk(output_path, record)
+            joined = record.walk_seq
+            # Shielded: a client that goes away never cancels a shared walk.
+            outcome = await asyncio.shield(flight)
+            if joined < needed:
+                continue  # an older walk ended; Rescan needs a newer one
+            if outcome.kind == FAILED:
+                raise outcome.error
+            if outcome.kind == DISCARDED:
+                discarded += 1
+                if discarded >= MAX_DISCARDED_WALKS:
+                    raise RuntimeError(
+                        "The output listing kept changing while it was read"
+                    )
+
+    def _refresh_if_stale(self, output_path, record):
+        age = _time.monotonic() - (record.walked_at or 0.0)
+        if age >= gallery_listing.REFRESH_AFTER:
+            self._schedule_walk(output_path)
+
+    def _schedule_walk(self, output_path):
+        """Start a background walk of *output_path* unless one is running."""
+        record = self._root_listing(output_path)
+        if record.flight is None:
+            self._start_walk(output_path, record)
+
+    def _start_walk(self, output_path, record):
+        """Start the root's one walk; returns its flight future (on the loop)."""
+        loop = asyncio.get_running_loop()
+        record.walk_seq += 1
+        flight = loop.create_future()
+        record.flight = flight
+        task = loop.create_task(
+            self._run_walk(
+                output_path, record, flight, record.generation, record.walk_seq
+            )
+        )
+        self._listing_tasks.add(task)
+        task.add_done_callback(self._listing_tasks.discard)
+        return flight
+
+    async def _run_walk(self, output_path, record, flight, generation, seq):
+        """Walk in the executor, then store, discard or fail (on the loop).
+
+        The flight is always resolved with a WalkOutcome, never failed, so an
+        error never reaches a future nobody awaits.
+        """
+        outcome = None
+        try:
+            files, folders, with_ancestors = await self._run_in_executor(
+                walk_root, self._scan_gallery_files_sync, output_path, record.files
+            )
+        except asyncio.CancelledError:
+            outcome = WalkOutcome.failed(RuntimeError(WALK_STOPPED))
+            raise
+        except Exception as exc:
+            record.walked_at = _time.monotonic()
+            if not record.failing:
+                self.logger.warning(
+                    f"Could not list output folder {output_path}: {exc}"
+                )
+            record.failing = True
+            outcome = WalkOutcome.failed(exc)
+        else:
+            record.failing = False
+            if record.generation == generation:
+                record.files = files
+                record.folders = folders
+                record.folders_with_ancestors = with_ancestors
+                record.walked_at = _time.monotonic()
+                record.stored_seq = seq
+                outcome = WalkOutcome.stored()
+            else:
+                outcome = WalkOutcome.discarded()
+        finally:
+            if record.flight is flight:
+                record.flight = None
+            if not flight.done():
+                flight.set_result(outcome)
 
     async def get_output_images(self, request):
         """Get all images from ComfyUI output folder(s)."""
@@ -1832,17 +1932,18 @@ class ImageRoutesMixin:
             )
 
     async def rescan_gallery(self, request):
-        """Forget the cached listing and rewalk every output root now.
+        """Rewalk every output root now and answer with the new listing.
 
-        The listing is cached for ``_gallery_cache_ttl`` seconds; the gallery's
-        Rescan button needs files added or removed on disk to show up at once.
+        Readers are served the shared listing and refresh it in the
+        background (``gallery_listing.REFRESH_AFTER``); the gallery's Rescan
+        button needs files added or removed on disk to show up at once, so it
+        waits for a walk that started after the click and fails when it does.
         """
         try:
-            self.invalidate_gallery_cache()
             output_dirs = list(self._get_all_output_dirs())
             total = 0
             for output_path in output_dirs:
-                total += len(await self._get_gallery_files(output_path))
+                total += len((await self._listing(output_path, fresh=True)).files)
             return web.json_response(
                 {"success": True, "total": total, "roots": len(output_dirs)}
             )
@@ -1853,20 +1954,14 @@ class ImageRoutesMixin:
             )
 
     async def _root_subfolders(self, output_path, include_ancestors):
-        """The distinct folders holding listed files under one output root."""
-        subfolders = set()
-        for f, _mtime in await self._get_gallery_files(output_path):
-            rel_dir = str(f.relative_to(output_path).parent)
-            if rel_dir and rel_dir != ".":
-                subfolders.add(rel_dir)
+        """The distinct folders holding listed files under one output root.
+
+        Built with the walk, in the executor (gallery_listing.folder_sets).
+        """
+        listing = await self._listing(output_path)
         if include_ancestors:
-            ancestors = set()
-            for folder in subfolders:
-                parts = folder.replace("\\", "/").split("/")
-                for i in range(1, len(parts)):
-                    ancestors.add("/".join(parts[:i]))
-            subfolders.update(ancestors)
-        return subfolders
+            return listing.folders_with_ancestors
+        return listing.folders
 
     async def get_gallery_subfolders(self, request):
         """Get distinct subfolders from gallery output directories.
@@ -1940,7 +2035,6 @@ class ImageRoutesMixin:
             result = await self._run_in_executor(
                 self._generate_gallery_thumbnails_sync, output_dirs
             )
-            self.invalidate_gallery_cache()
             return web.json_response(result)
 
         except Exception as e:
@@ -2416,7 +2510,6 @@ class ImageRoutesMixin:
             cleared_count, cleared_size = await self._run_in_executor(
                 self._clear_gallery_thumbnails_sync, output_dirs
             )
-            self.invalidate_gallery_cache()
 
             def format_size(bytes_size: int) -> str:
                 size = float(bytes_size)

py/api/thumbnail_convert.py

@@ -175,7 +175,6 @@ class ThumbnailConvertMixin:
             job.log_line(f"Conversion finished · {NOTHING_OUTDATED.lower()}")
             return "finished"
         totals, left = await self._convert_in_batches(job, options, plan, output_dirs)
-        self.invalidate_gallery_cache()
         if totals["stopped"]:
             job.update(status_line=FORMAT_CHANGED.format(converted=totals["converted"]))
             job.log_line(

py/api/thumbnail_jobs.py

@@ -261,7 +261,6 @@ class ThumbnailJobsMixin:
         stats, start, left, attempted = await self._generate_in_batches(
             job, options, plan, roots, output_dirs
         )
-        self.invalidate_gallery_cache()
         generated = _plural(stats["generated"], "preview")
         if job.cancelled:
             return self._end_cancelled(