mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-09-21 11:11:26 -03:00
perf(services): skip per-file realpath work in cache reconciliation
A no-change Refresh still computed os.path.realpath for every model file in the library and for every cached entry. Both values are only ever consulted when a discovered file is missing from the cache, so on a 50k-file library they cost ~1.3s and ~0.6s while being used zero times. - Compute the per-file realpath only after the exact cache match fails - Build the physical-path alias map lazily on the first miss; the cross-run alias guard (overlapping roots / symlink layout changes) still keeps the cached entry instead of a delete + re-add, which would re-read metadata and re-hash the whole library - Snapshot get_model_roots() once for the new-file pass instead of re-reading it for every added file - Run the duplicate-path integrity pass only when the snapshot already contained duplicates or files were appended; a clean, unchanged cache has nothing to clean. Duplicates can only be introduced by external code rewriting raw_data or by this pass's own appends. Zero-change reconcile drops from ~1400ms to ~120ms on 50k files, and an alias flip still re-processes 0 files (#1108 investigation).
This commit is contained in:
@@ -1005,14 +1005,29 @@ class ModelScanner:
|
|||||||
await self._broadcast_scan_progress('started', 'reconcile_scan', 0, False)
|
await self._broadcast_scan_progress('started', 'reconcile_scan', 0, False)
|
||||||
|
|
||||||
# Get current cached file paths
|
# Get current cached file paths
|
||||||
|
cached_size_before = len(self._cache.raw_data)
|
||||||
cached_paths = {item['file_path'] for item in self._cache.raw_data}
|
cached_paths = {item['file_path'] for item in self._cache.raw_data}
|
||||||
path_to_item = {item['file_path']: item for item in self._cache.raw_data}
|
path_to_item = {item['file_path']: item for item in self._cache.raw_data}
|
||||||
|
|
||||||
|
# physical path -> cached business path, for the alias case where the
|
||||||
|
# same file is reachable under a different path than the cached one
|
||||||
|
# (overlapping roots / symlink layout changes): keep the existing
|
||||||
|
# entry instead of delete + re-add (which would re-read metadata and
|
||||||
|
# re-hash every file). Built lazily on the first miss, because a
|
||||||
|
# realpath per cached entry is ~half the cost of a no-change
|
||||||
|
# reconcile and the map is only ever consulted for misses.
|
||||||
|
cached_real_paths: Optional[Dict[str, str]] = None
|
||||||
|
|
||||||
|
def lookup_cached_real_path(real_path: str) -> Optional[str]:
|
||||||
|
nonlocal cached_real_paths
|
||||||
|
if cached_real_paths is None:
|
||||||
cached_real_paths = {}
|
cached_real_paths = {}
|
||||||
for cached_path in cached_paths:
|
for cached_path in cached_paths:
|
||||||
try:
|
try:
|
||||||
cached_real_paths.setdefault(os.path.realpath(cached_path), cached_path)
|
cached_real_paths.setdefault(os.path.realpath(cached_path), cached_path)
|
||||||
except Exception:
|
except Exception:
|
||||||
continue
|
continue
|
||||||
|
return cached_real_paths.get(real_path)
|
||||||
|
|
||||||
# Track found files and new files
|
# Track found files and new files
|
||||||
found_paths = set()
|
found_paths = set()
|
||||||
@@ -1038,14 +1053,18 @@ class ModelScanner:
|
|||||||
if ext in self.file_extensions:
|
if ext in self.file_extensions:
|
||||||
# Construct paths exactly as they would be in cache
|
# Construct paths exactly as they would be in cache
|
||||||
file_path = os.path.join(root, file).replace(os.sep, '/')
|
file_path = os.path.join(root, file).replace(os.sep, '/')
|
||||||
real_file_path = os.path.realpath(os.path.join(root, file))
|
|
||||||
|
|
||||||
# Check if this file is already in cache
|
# Check if this file is already in cache
|
||||||
if file_path in cached_paths:
|
if file_path in cached_paths:
|
||||||
found_paths.add(file_path)
|
found_paths.add(file_path)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
cached_real_match = cached_real_paths.get(real_file_path)
|
# Only a cache miss needs the physical path, so the
|
||||||
|
# realpath syscalls are paid per changed file rather
|
||||||
|
# than per file in the library.
|
||||||
|
real_file_path = os.path.realpath(os.path.join(root, file))
|
||||||
|
|
||||||
|
cached_real_match = lookup_cached_real_path(real_file_path)
|
||||||
if cached_real_match:
|
if cached_real_match:
|
||||||
found_paths.add(cached_real_match)
|
found_paths.add(cached_real_match)
|
||||||
continue
|
continue
|
||||||
@@ -1090,6 +1109,9 @@ class ModelScanner:
|
|||||||
total_new = len(new_files)
|
total_new = len(new_files)
|
||||||
processed_new = 0
|
processed_new = 0
|
||||||
last_progress_time = time.time()
|
last_progress_time = time.time()
|
||||||
|
# Snapshot the roots once: this matches the walk above (which
|
||||||
|
# also snapshots them) and avoids a config read per new file.
|
||||||
|
model_roots = self.get_model_roots()
|
||||||
for i in range(0, total_new, batch_size):
|
for i in range(0, total_new, batch_size):
|
||||||
batch = new_files[i:i+batch_size]
|
batch = new_files[i:i+batch_size]
|
||||||
for path in batch:
|
for path in batch:
|
||||||
@@ -1098,12 +1120,10 @@ class ModelScanner:
|
|||||||
try:
|
try:
|
||||||
# Find the appropriate root path for this file
|
# Find the appropriate root path for this file
|
||||||
root_path = None
|
root_path = None
|
||||||
model_roots = self.get_model_roots()
|
normalized_path = os.path.normpath(path)
|
||||||
for potential_root in model_roots:
|
for potential_root in model_roots:
|
||||||
# Normalize both paths for comparison
|
# Normalize both paths for comparison
|
||||||
normalized_path = os.path.normpath(path)
|
if normalized_path.startswith(os.path.normpath(potential_root)):
|
||||||
normalized_root = os.path.normpath(potential_root)
|
|
||||||
if normalized_path.startswith(normalized_root):
|
|
||||||
root_path = potential_root
|
root_path = potential_root
|
||||||
break
|
break
|
||||||
|
|
||||||
@@ -1200,6 +1220,14 @@ class ModelScanner:
|
|||||||
# Update cache data
|
# Update cache data
|
||||||
self._cache.raw_data = [item for item in self._cache.raw_data if item['file_path'] not in missing_files]
|
self._cache.raw_data = [item for item in self._cache.raw_data if item['file_path'] not in missing_files]
|
||||||
|
|
||||||
|
# Defensive integrity pass: drop entries sharing a business path.
|
||||||
|
# Duplicates can only be introduced by external code rewriting
|
||||||
|
# raw_data directly or by this pass's own appends, so an unchanged
|
||||||
|
# filesystem walk over a clean cache has nothing to clean. The size
|
||||||
|
# mismatch is an O(1) tell that the snapshot already contained
|
||||||
|
# duplicates; skipping the O(N) pass when it is provably clean is
|
||||||
|
# what keeps a no-change Refresh cheap.
|
||||||
|
if cached_size_before != len(cached_paths) or total_added > 0:
|
||||||
dedup_removed = 0
|
dedup_removed = 0
|
||||||
seen_paths: set[str] = set()
|
seen_paths: set[str] = set()
|
||||||
deduped: list[Dict[str, Any]] = []
|
deduped: list[Dict[str, Any]] = []
|
||||||
|
|||||||
@@ -732,6 +732,130 @@ async def test_reconcile_cache_removes_duplicate_alias_when_same_real_file_seen_
|
|||||||
assert cached_paths == {_normalize_path(loras_root / "link" / "one.txt")}
|
assert cached_paths == {_normalize_path(loras_root / "link" / "one.txt")}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_reconcile_cache_keeps_cached_path_when_walk_yields_a_live_alias(
|
||||||
|
tmp_path: Path,
|
||||||
|
):
|
||||||
|
"""A root-order / symlink change can make the walk produce a *different but
|
||||||
|
still live* business path for a file already in the cache. The realpath
|
||||||
|
alias map must keep the cached entry instead of re-processing the file and
|
||||||
|
swapping the path (which would re-read metadata and re-hash the weights)."""
|
||||||
|
loras_root = tmp_path / "loras"
|
||||||
|
loras_root.mkdir()
|
||||||
|
extra_root = tmp_path / "extra"
|
||||||
|
extra_root.mkdir()
|
||||||
|
(extra_root / "one.txt").write_text("one", encoding="utf-8")
|
||||||
|
(loras_root / "link").symlink_to(extra_root, target_is_directory=True)
|
||||||
|
|
||||||
|
# `extra_root` comes first, so the cache entry is stored under its path.
|
||||||
|
scanner = MultiRootDummyScanner([extra_root, loras_root])
|
||||||
|
await scanner._initialize_cache()
|
||||||
|
|
||||||
|
cached_before = {item["file_path"] for item in scanner._cache.raw_data}
|
||||||
|
assert cached_before == {_normalize_path(extra_root / "one.txt")}
|
||||||
|
|
||||||
|
# The symlinked path now wins the walk; the file itself is unchanged.
|
||||||
|
scanner._roots = [str(loras_root), str(extra_root)]
|
||||||
|
processed: List[str] = []
|
||||||
|
|
||||||
|
async def _record_process(file_path: str, root_path: str, *args, **kwargs):
|
||||||
|
processed.append(file_path)
|
||||||
|
return await DummyScanner._process_model_file(
|
||||||
|
scanner, file_path, root_path, *args, **kwargs
|
||||||
|
)
|
||||||
|
|
||||||
|
scanner._process_model_file = _record_process # type: ignore[method-assign]
|
||||||
|
|
||||||
|
await scanner._reconcile_cache()
|
||||||
|
|
||||||
|
cache = await scanner.get_cached_data()
|
||||||
|
assert {item["file_path"] for item in cache.raw_data} == cached_before
|
||||||
|
assert processed == []
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_reconcile_cache_defers_realpath_to_cache_misses(
|
||||||
|
tmp_path: Path, monkeypatch
|
||||||
|
):
|
||||||
|
"""A no-change reconcile must not call realpath for unchanged files or for
|
||||||
|
every cached entry: both the alias map and the per-file realpath are only
|
||||||
|
needed for cache misses (they dominate the cost of a Refresh otherwise)."""
|
||||||
|
root = tmp_path / "loras"
|
||||||
|
root.mkdir()
|
||||||
|
for i in range(5):
|
||||||
|
(root / f"model{i}.txt").write_text("x", encoding="utf-8")
|
||||||
|
|
||||||
|
scanner = DummyScanner(root)
|
||||||
|
await scanner._initialize_cache()
|
||||||
|
|
||||||
|
real_realpath = model_scanner.os.path.realpath
|
||||||
|
realpath_args: List[str] = []
|
||||||
|
|
||||||
|
def _recording_realpath(path, *args, **kwargs):
|
||||||
|
realpath_args.append(os.fspath(path))
|
||||||
|
return real_realpath(path, *args, **kwargs)
|
||||||
|
|
||||||
|
monkeypatch.setattr(model_scanner.os.path, "realpath", _recording_realpath)
|
||||||
|
|
||||||
|
await scanner._reconcile_cache()
|
||||||
|
|
||||||
|
model_files = {_normalize_path(path) for path in root.glob("*.txt")}
|
||||||
|
assert not (set(realpath_args) & model_files)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_reconcile_cache_cleans_pre_existing_duplicate_paths(tmp_path: Path):
|
||||||
|
"""External code rewrites raw_data directly, so a reconcile must still drop
|
||||||
|
duplicate business paths even when nothing changed on disk: the O(1)
|
||||||
|
integrity check may only skip the pass for a provably clean cache."""
|
||||||
|
root = tmp_path / "loras"
|
||||||
|
root.mkdir()
|
||||||
|
(root / "one.txt").write_text("one", encoding="utf-8")
|
||||||
|
(root / "two.txt").write_text("two", encoding="utf-8")
|
||||||
|
|
||||||
|
scanner = DummyScanner(root)
|
||||||
|
await scanner._initialize_cache()
|
||||||
|
|
||||||
|
first_path = _normalize_path(root / "one.txt")
|
||||||
|
duplicate = dict(next(i for i in scanner._cache.raw_data if i["file_path"] == first_path))
|
||||||
|
duplicate["model_name"] = "duplicate-wins"
|
||||||
|
scanner._cache.raw_data.append(duplicate)
|
||||||
|
|
||||||
|
await scanner._reconcile_cache()
|
||||||
|
|
||||||
|
cache = await scanner.get_cached_data()
|
||||||
|
assert len(cache.raw_data) == 2
|
||||||
|
survivor = next(i for i in cache.raw_data if i["file_path"] == first_path)
|
||||||
|
assert survivor["model_name"] == "duplicate-wins"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_reconcile_cache_reads_model_roots_once_per_phase(tmp_path: Path, monkeypatch):
|
||||||
|
"""get_model_roots() must be snapshotted once for the walk and once for the
|
||||||
|
new-file pass, not re-read for every new file."""
|
||||||
|
root = tmp_path / "loras"
|
||||||
|
root.mkdir()
|
||||||
|
scanner = DummyScanner(root)
|
||||||
|
await scanner._initialize_cache()
|
||||||
|
|
||||||
|
calls = 0
|
||||||
|
real_get_model_roots = scanner.get_model_roots
|
||||||
|
|
||||||
|
def _counting_get_model_roots() -> List[str]:
|
||||||
|
nonlocal calls
|
||||||
|
calls += 1
|
||||||
|
return real_get_model_roots()
|
||||||
|
|
||||||
|
monkeypatch.setattr(scanner, "get_model_roots", _counting_get_model_roots)
|
||||||
|
|
||||||
|
for i in range(3):
|
||||||
|
(root / f"new{i}.txt").write_text("x", encoding="utf-8")
|
||||||
|
|
||||||
|
await scanner._reconcile_cache()
|
||||||
|
|
||||||
|
assert calls == 2
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_log_duplicate_filename_summary_logs_warning(tmp_path: Path, caplog):
|
async def test_log_duplicate_filename_summary_logs_warning(tmp_path: Path, caplog):
|
||||||
"""When duplicate filenames exist, _log_duplicate_filename_summary should emit
|
"""When duplicate filenames exist, _log_duplicate_filename_summary should emit
|
||||||
|
|||||||
Reference in New Issue
Block a user