perf(images): refresh the listing when the monitor sees a new output (#348, PR 2)
The image monitor now reports every settled output (linked or not, also when linking raised) with its resolved path through set_output_listener. The API's notify_output_settled hops from the monitor's worker thread onto the server's loop (call_soon_threadsafe) and does nothing before the loop is known or after cleanup. mark_root_dirty matches the path against each walk's resolved_root (normcase(realpath), computed in the executor by walk_root, now its fourth value) and marks the root dirty. A dirty root gets one background walk, no sooner than rewalk_interval = min(15, max(5, 5 x last walk's duration)) after the previous walk started (last_started_at / last_duration are recorded on every walk). _schedule_walk is shared with PR 1's delayed stale refresh: one deferred walk per root, never armed while a walk runs (the walk's end re-checks dirty: the trailing walk), at least 0.5 s away, the earlier deadline wins, and every walk start cancels it. A failed walk clears dirty. Rescan semantics are unchanged.
This commit is contained in:
+24
-1
@@ -362,8 +362,11 @@ class PromptManagerAPI(
|
||||
# background walks refreshing them (py/api/gallery_listing.py, #348).
|
||||
self._root_listings: dict[str, RootListing] = {}
|
||||
self._listing_tasks: set = set()
|
||||
# The loop the walks run on, set by the first walk.
|
||||
# The loop the walks run on, set by the startup warm-up or the first
|
||||
# walk; the monitor's thread hops onto it (notify_output_settled).
|
||||
self._listing_loop = None
|
||||
# Set by cleanup_gallery_listings: no background walk starts after it.
|
||||
self._listings_closed = False
|
||||
# The workspace caches rebuilt behind a stored walk that changed the
|
||||
# listing, once the host's startup warm-up has run (#348).
|
||||
self._workspace_warm_enabled = False
|
||||
@@ -539,15 +542,35 @@ class PromptManagerAPI(
|
||||
# Each stored walk that changes a listing then rebuilds the workspace's
|
||||
# per-file values, orders and favorites behind it (_warm_workspace).
|
||||
self._workspace_warm_enabled = True
|
||||
self._listing_loop = asyncio.get_running_loop()
|
||||
self._listings_closed = False
|
||||
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}")
|
||||
|
||||
def notify_output_settled(self, path):
|
||||
"""The image monitor settled an output at *path* (any thread).
|
||||
|
||||
Called on the monitor's worker thread with the resolved path. It only
|
||||
hands the path to the event loop (``mark_root_dirty``) and never
|
||||
touches a listing from this thread; before the loop is known, or once
|
||||
the listings are cleaned up, it does nothing.
|
||||
"""
|
||||
loop = self._listing_loop
|
||||
if loop is None or self._listings_closed or not loop.is_running():
|
||||
return
|
||||
try:
|
||||
loop.call_soon_threadsafe(self.mark_root_dirty, path)
|
||||
except RuntimeError:
|
||||
pass # the loop closed meanwhile (shutdown)
|
||||
|
||||
async def cleanup_gallery_listings(self, app=None):
|
||||
"""Stop the background listing walks (aiohttp on_cleanup)."""
|
||||
self._workspace_warm_enabled = False
|
||||
# No mark, deferred walk or trailing walk starts a walk from now on.
|
||||
self._listings_closed = True
|
||||
# Before any await, so no delayed refresh can start a walk meanwhile.
|
||||
for record in self._root_listings.values():
|
||||
handle, record.rewalk_handle = record.rewalk_handle, None
|
||||
|
||||
@@ -6,6 +6,10 @@ 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.
|
||||
|
||||
A settled output the image monitor reports marks its root dirty
|
||||
(``mark_root_dirty``); a dirty root gets one background walk, no sooner than
|
||||
``rewalk_interval`` after its previous walk started (PR 2).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -26,6 +30,18 @@ REFRESH_AFTER = 30
|
||||
# (#348 live test: reads during the walk took 2.2 s instead of 0.07 s).
|
||||
REFRESH_DELAY = 1.0
|
||||
|
||||
# No background walk of a root starts sooner than rewalk_interval(last walk's
|
||||
# duration) after its previous walk started: FACTOR times that duration, at
|
||||
# least FLOOR and at most CAP seconds. Background walks then hold the GIL at
|
||||
# most ~20% of the time; the cap keeps a new output listed within ~20 s on a
|
||||
# slow machine. Read through this module so tests can patch them.
|
||||
REWALK_FLOOR = 5
|
||||
REWALK_FACTOR = 5
|
||||
REWALK_CAP = 15
|
||||
|
||||
# A deferred walk is never armed sooner than this: no zero-delay loop.
|
||||
MIN_REWALK_DELAY = 0.5
|
||||
|
||||
# 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
|
||||
@@ -80,8 +96,17 @@ class RootListing:
|
||||
flight: asyncio.Future | None = None
|
||||
# True while walks keep failing: one warning per streak.
|
||||
failing: bool = False
|
||||
# The delayed refresh not started yet (loop.call_later), or None.
|
||||
# The one deferred background walk not started yet (loop.call_later), or
|
||||
# None: a stale refresh or a dirty root's rewalk, whichever is due first.
|
||||
rewalk_handle: asyncio.TimerHandle | None = None
|
||||
# normcase(realpath(root)), computed by the walk in the executor: monitor
|
||||
# paths are matched against it without file I/O on the loop.
|
||||
resolved_root: str | None = None
|
||||
# Monotonic start and duration of the last walk (REWALK_MIN_INTERVAL).
|
||||
last_started_at: float | None = None
|
||||
last_duration: float | None = None
|
||||
# A settled output was reported under this root; cleared when a walk starts.
|
||||
dirty: bool = False
|
||||
|
||||
def drop(self) -> None:
|
||||
self.files = None
|
||||
@@ -121,6 +146,34 @@ def folder_sets(root, files) -> tuple[frozenset, frozenset]:
|
||||
return frozenset(folders), frozenset(with_ancestors)
|
||||
|
||||
|
||||
def rewalk_interval(last_duration) -> float:
|
||||
"""REWALK_MIN_INTERVAL after a walk that took *last_duration* seconds."""
|
||||
scaled = REWALK_FACTOR * (last_duration or 0.0)
|
||||
return min(REWALK_CAP, max(REWALK_FLOOR, scaled))
|
||||
|
||||
|
||||
def root_contains(resolved_root: str, path, pathmod=os.path) -> bool:
|
||||
"""True when the resolved *path* lies under *resolved_root* (no file I/O).
|
||||
|
||||
*resolved_root* is already ``normcase``d; *path* is folded the same way,
|
||||
so Windows matches case-insensitively and with either separator. The
|
||||
match ends at a separator: ``/out2/x.png`` is not under ``/out``.
|
||||
"""
|
||||
target = pathmod.normcase(str(path))
|
||||
prefix = resolved_root
|
||||
if not prefix.endswith(pathmod.sep):
|
||||
prefix += pathmod.sep
|
||||
return target.startswith(prefix)
|
||||
|
||||
|
||||
def resolve_root(root) -> str:
|
||||
"""``normcase(realpath(root))``, the spelling monitor paths are matched in.
|
||||
|
||||
File I/O (realpath follows links): call it in the executor.
|
||||
"""
|
||||
return os.path.normcase(os.path.realpath(str(root)))
|
||||
|
||||
|
||||
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:
|
||||
@@ -130,8 +183,10 @@ def same_listing(old, new) -> bool:
|
||||
|
||||
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)``.
|
||||
) -> tuple[list, frozenset, frozenset, str]:
|
||||
"""The single executor job of a walk.
|
||||
|
||||
Returns ``(files, folders, with_ancestors, resolved_root)``.
|
||||
|
||||
Opens the root first, so a missing or unreadable root raises ``OSError``
|
||||
instead of listing nothing. Returns *previous_files* itself when nothing
|
||||
@@ -143,4 +198,4 @@ def walk_root(
|
||||
if same_listing(previous_files, files):
|
||||
files = previous_files
|
||||
folders, with_ancestors = folder_sets(root, files)
|
||||
return files, folders, with_ancestors
|
||||
return files, folders, with_ancestors, resolve_root(root)
|
||||
|
||||
+79
-15
@@ -1433,28 +1433,81 @@ class ImageRoutesMixin:
|
||||
)
|
||||
|
||||
def _refresh_if_stale(self, output_path, record):
|
||||
"""Walk a stale listing again, REFRESH_DELAY after this read (on the loop).
|
||||
"""Ask for a background walk when the listing is stale or dirty.
|
||||
|
||||
At most one pending refresh per root, none while a walk runs.
|
||||
A stale listing (older than REFRESH_AFTER) is walked REFRESH_DELAY
|
||||
after this read, so the walk does not share the GIL with the page's
|
||||
own requests; a dirty one as soon as its interval allows.
|
||||
"""
|
||||
age = _time.monotonic() - (record.walked_at or 0.0)
|
||||
if age < gallery_listing.REFRESH_AFTER:
|
||||
if age >= gallery_listing.REFRESH_AFTER:
|
||||
self._schedule_walk(output_path, gallery_listing.REFRESH_DELAY)
|
||||
elif record.dirty:
|
||||
self._schedule_walk(output_path)
|
||||
|
||||
def mark_root_dirty(self, path):
|
||||
"""A settled output at *path*: walk every root holding it again (on the loop).
|
||||
|
||||
*path* is resolved by the monitor's thread (``notify_output_settled``),
|
||||
so this does no file I/O: it compares ``normcase`` strings with each
|
||||
walk's ``resolved_root``. Only roots with a listing, or a walk running,
|
||||
are marked; a root without either is walked by its next read anyway.
|
||||
"""
|
||||
if self._listings_closed:
|
||||
return
|
||||
if record.flight is not None or record.rewalk_handle is not None:
|
||||
for key, record in list(self._root_listings.items()):
|
||||
if record.files is None and record.flight is None:
|
||||
continue
|
||||
resolved = record.resolved_root or os.path.normcase(key)
|
||||
if gallery_listing.root_contains(resolved, path):
|
||||
record.dirty = True
|
||||
self._schedule_walk(Path(key))
|
||||
|
||||
def _schedule_walk(self, output_path, delay=0.0):
|
||||
"""Start or defer a background walk of *output_path* (on the loop).
|
||||
|
||||
- A walk is running: nothing; its end re-checks ``dirty``.
|
||||
- Otherwise the walk is due *delay* from now, and never sooner than
|
||||
``rewalk_interval`` after the previous walk started. Due now: start
|
||||
it. Later: one deferred walk per root, at least MIN_REWALK_DELAY
|
||||
away; when one is already pending, the earlier deadline wins.
|
||||
Every walk start cancels the pending one (``_start_walk``).
|
||||
"""
|
||||
if self._listings_closed:
|
||||
return
|
||||
record.rewalk_handle = asyncio.get_running_loop().call_later(
|
||||
gallery_listing.REFRESH_DELAY, self._delayed_walk, output_path, record
|
||||
record = self._root_listing(output_path)
|
||||
if record.flight is not None:
|
||||
return
|
||||
now = _time.monotonic()
|
||||
due = now + delay
|
||||
if record.last_started_at is not None:
|
||||
interval = gallery_listing.rewalk_interval(record.last_duration)
|
||||
due = max(due, record.last_started_at + interval)
|
||||
if due <= now:
|
||||
self._start_walk(output_path, record)
|
||||
return
|
||||
wait = max(gallery_listing.MIN_REWALK_DELAY, due - now)
|
||||
loop = asyncio.get_running_loop()
|
||||
handle = record.rewalk_handle
|
||||
if handle is not None:
|
||||
if handle.when() - loop.time() <= wait:
|
||||
return # the pending walk comes first
|
||||
handle.cancel()
|
||||
record.rewalk_handle = loop.call_later(
|
||||
wait, self._delayed_walk, output_path, record
|
||||
)
|
||||
|
||||
def _delayed_walk(self, output_path, record):
|
||||
record.rewalk_handle = None
|
||||
self._schedule_walk(output_path)
|
||||
"""The deferred walk's timer: clear the handle first, then walk.
|
||||
|
||||
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)
|
||||
It was armed for a stale or dirty listing at the earliest moment the
|
||||
interval allowed. Only a walk start clears either reason, and every
|
||||
start cancels this timer, so when it fires the walk is still due.
|
||||
"""
|
||||
record.rewalk_handle = None
|
||||
if self._listings_closed or record.flight is not None:
|
||||
return
|
||||
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)."""
|
||||
@@ -1462,7 +1515,10 @@ class ImageRoutesMixin:
|
||||
self._listing_loop = loop
|
||||
handle, record.rewalk_handle = record.rewalk_handle, None
|
||||
if handle is not None:
|
||||
handle.cancel() # this walk is the refresh it was waiting to start
|
||||
handle.cancel() # this walk is the one it was waiting to start
|
||||
# This walk lists every output reported so far.
|
||||
record.dirty = False
|
||||
record.last_started_at = _time.monotonic()
|
||||
record.walk_seq += 1
|
||||
flight = loop.create_future()
|
||||
record.flight = flight
|
||||
@@ -1482,8 +1538,9 @@ class ImageRoutesMixin:
|
||||
error never reaches a future nobody awaits.
|
||||
"""
|
||||
outcome = None
|
||||
started = record.last_started_at
|
||||
try:
|
||||
files, folders, with_ancestors = await self._run_in_executor(
|
||||
files, folders, with_ancestors, resolved = await self._run_in_executor(
|
||||
walk_root, self._scan_gallery_files_sync, output_path, record.files
|
||||
)
|
||||
except asyncio.CancelledError:
|
||||
@@ -1496,9 +1553,12 @@ class ImageRoutesMixin:
|
||||
f"Could not list output folder {output_path}: {exc}"
|
||||
)
|
||||
record.failing = True
|
||||
# Retried by the 30 s rule, not by a trailing walk.
|
||||
record.dirty = False
|
||||
outcome = WalkOutcome.failed(exc)
|
||||
else:
|
||||
record.failing = False
|
||||
record.resolved_root = resolved
|
||||
if record.generation == generation:
|
||||
changed = files is not record.files
|
||||
record.files = files
|
||||
@@ -1512,10 +1572,14 @@ class ImageRoutesMixin:
|
||||
else:
|
||||
outcome = WalkOutcome.discarded()
|
||||
finally:
|
||||
record.last_duration = _time.monotonic() - started
|
||||
if record.flight is flight:
|
||||
record.flight = None
|
||||
if not flight.done():
|
||||
flight.set_result(outcome)
|
||||
if record.dirty:
|
||||
# An output settled during this walk: the trailing walk.
|
||||
self._schedule_walk(output_path)
|
||||
|
||||
async def get_output_images(self, request):
|
||||
"""Get all images from ComfyUI output folder(s)."""
|
||||
|
||||
Reference in New Issue
Block a user