perf(assets): replace pathlib prefix checks in the asset scan (#16543) · code/app-python-comfyui@7897b4e · GitHub
Skip to content

Commit 7897b4e

Browse files
synap5eguill
andauthored
perf(assets): replace pathlib prefix checks in the asset scan (Comfy-Org#16543)
* Speed up the startup prune's owned-prefix check mark_contents_missing_outside_prefixes tested every live AssetContent row against every owned prefix with Path.is_relative_to, which walks the path's parents on each call: rows x prefixes x depth. At 9k rows and 150 prefixes that was 9.5s (15.5s with deeper output paths). path_prefix_matcher normalizes the prefixes once and checks each row with a normcase'd, separator-bounded str.startswith over a tuple, keeping is_relative_to's component bounds and platform case rules. The same case now takes about 0.04s, flat in prefix count and depth. * Move path_prefix_matcher next to the SQL containment predicate path_utils needs the same check for per-file tagging, and scanner_changes already imports path_utils, so the matcher moves to app.assets.helpers beside sql_path_under_prefix. * Speed up asset tag derivation during scans get_backend_system_tags_from_path checked every scanned file against every model base with Path.is_relative_to, which dominated the fast scan: 37s of build_asset_specs for 100k output files. Using path_prefix_matcher, the separator-bounded string check the startup prune already uses, brings that to about 7s. * Keep path_prefix_matcher's parity for paths starting with two slashes abspath keeps exactly two leading slashes, and pathlib treats that '//' as an anchor of its own, so Path('//server/f').is_relative_to('/') is False, but the string check accepted it. Prefixes are now split once by whether their anchor is '//', and a candidate is only checked against prefixes with the same anchor. The per-row check is still a single str.startswith. * Build tag-derivation prefix matchers once per folder config get_backend_system_tags_from_path built a path_prefix_matcher for input, output, temp and every model category on each call, so most of its cost was normalizing the same prefixes again for every scanned file. cached_prefix_matcher memoizes construction on the raw prefix tuple, which the callers pass as absolute folder paths; a folder-config change is a new key. 100k files, 58 model bases: tag derivation 81.8 -> 27.7 us/file, and build_asset_specs CPU 11.55 -> 6.40s. * State cached_prefix_matcher's absolute-prefix precondition The docstring presented absolute prefixes as a property of the callers. It is a precondition the caller must meet: folder_paths stores what it is given, and a relative prefix is resolved once, on first use, and then frozen. --------- Co-authored-by: guill <jacob.e.segal@gmail.com>
1 parent 66b68a3 commit 7897b4e

7 files changed

Lines changed: 155 additions & 25 deletions

File tree

‎app/assets/helpers.py‎

Lines changed: 53 additions & 3 deletions

‎app/assets/scanner.py‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,12 @@
2727
create_record,
2828
)
2929
from app.assets.database.models import Asset, AssetContent
30-
from app.assets.helpers import sql_path_under_prefix, to_stored_hash
30+
from app.assets.helpers import path_prefix_matcher, sql_path_under_prefix, to_stored_hash
3131
from app.assets.lifecycle import get_excluded_scan_roots
3232
from app.assets.scanner_changes import (
3333
clear_pending_verifications,
3434
detect_content_change,
3535
drain_pending_verifications,
36-
is_path_under_prefixes,
3736
live_contents_under_prefixes,
3837
pending_recovery_count,
3938
recover_missing_content,
@@ -240,10 +239,6 @@ def _sync_prefixes_in_write_txn(
240239
return survivors
241240

242241

243-
def _is_under_prefixes(path: str, prefixes: list[str]) -> bool:
244-
return is_path_under_prefixes(path, prefixes)
245-
246-
247242
def sync_root_safely(
248243
root: RootType, progress: _ScanProgress | None = None
249244
) -> set[str]:
@@ -304,7 +299,8 @@ def mark_contents_missing_outside_prefixes(
304299
contents = session.scalars(
305300
sa.select(AssetContent).where(AssetContent.is_missing.is_(False))
306301
)
307-
missing = [content for content in contents if not _is_under_prefixes(content.path, prefixes)]
302+
is_owned = path_prefix_matcher(prefixes)
303+
missing = [content for content in contents if not is_owned(content.path)]
308304
for content in missing:
309305
mark_content_missing(session, content.id)
310306
return len(missing)

‎app/assets/scanner_changes.py‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
from __future__ import annotations
99

1010
import os
11-
from pathlib import Path
1211
from typing import Literal
1312

1413
import sqlalchemy as sa
@@ -21,7 +20,7 @@
2120
mark_content_missing,
2221
unset_content_missing,
2322
)
24-
from app.assets.helpers import sql_path_under_prefix, to_stored_hash
23+
from app.assets.helpers import path_prefix_matcher, sql_path_under_prefix, to_stored_hash
2524
from app.assets.services.path_utils import compute_loader_path, get_name_and_tags_from_asset_path
2625
from app.assets.services.snapshot_hash import snapshot_hash
2726

@@ -108,8 +107,7 @@ def recover_missing_content(
108107

109108

110109
def is_path_under_prefixes(path: str, prefixes: list[str]) -> bool:
111-
candidate = Path(os.path.abspath(path))
112-
return any(candidate.is_relative_to(os.path.abspath(prefix)) for prefix in prefixes)
110+
return path_prefix_matcher(prefixes)(path)
113111

114112

115113
def split_content(session: Session, content: AssetContent, stat_result: os.stat_result, hash_value: str | None) -> AssetContent:

‎app/assets/services/path_utils.py‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from typing import Literal
44

55
import folder_paths
6+
from app.assets.helpers import cached_prefix_matcher
67

78

89
_NON_MODEL_FOLDER_NAMES = frozenset({"configs", "custom_nodes"})
@@ -250,7 +251,6 @@ def get_backend_system_tags_from_path(path: str) -> list[str]:
250251
category still get the ``models`` tag.
251252
"""
252253
fp_abs = os.path.abspath(path)
253-
fp_path = Path(fp_abs)
254254
tags: list[str] = []
255255

256256
def _add(tag: str) -> None:
@@ -262,21 +262,19 @@ def _add(tag: str) -> None:
262262
("output", folder_paths.get_output_directory()),
263263
("temp", folder_paths.get_temp_directory()),
264264
):
265-
if fp_path.is_relative_to(os.path.abspath(base)):
265+
if cached_prefix_matcher((base,))(fp_abs):
266266
_add(role)
267267

268268
ext = os.path.splitext(fp_abs)[1].lower()
269269
model_types: list[str] = []
270270
under_models_base = False
271271
for folder_name, bases, extensions in get_comfy_models_folders():
272-
for base in bases:
273-
if fp_path.is_relative_to(os.path.abspath(base)):
274-
under_models_base = True
275-
# Empty set accepts any extension, matching
276-
# folder_paths.filter_files_extensions semantics.
277-
if not extensions or ext in extensions:
278-
model_types.append(folder_name)
279-
break
272+
if cached_prefix_matcher(tuple(bases))(fp_abs):
273+
under_models_base = True
274+
# Empty set accepts any extension, matching
275+
# folder_paths.filter_files_extensions semantics.
276+
if not extensions or ext in extensions:
277+
model_types.append(folder_name)
280278

281279
if under_models_base:
282280
_add("models")
@@ -306,7 +304,7 @@ def get_known_input_subfolder_tags_from_path(path: str) -> list[str]:
306304
"""
307305
fp_abs = os.path.abspath(path)
308306
input_base = os.path.abspath(folder_paths.get_input_directory())
309-
if not Path(fp_abs).is_relative_to(input_base):
307+
if not cached_prefix_matcher((input_base,))(fp_abs):
310308
return []
311309

312310
rel = os.path.relpath(fp_abs, input_base)

‎tests-unit/assets_test/services/path_prefix_cases.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,19 @@ def prefix_case_paths(root: str) -> list[tuple[str, bool]]:
3333

3434
def expected_prefix_case_paths(root: str) -> set[str]:
3535
return {os.path.abspath(path) for path, expected in prefix_case_paths(root) if expected}
36+
37+
38+
def anchor_case_paths() -> list[tuple[str, str, bool]]:
39+
"""(path, prefix, Path(path).is_relative_to(prefix)) across POSIX anchors.
40+
41+
abspath keeps exactly two leading slashes, and pathlib treats "//" as its own
42+
anchor, so "//server/..." is not under "/". Three or more collapse to "/".
43+
"""
44+
return [
45+
("//server/file", "/", False),
46+
("//server/file", "//server", True),
47+
("//server", "//", True),
48+
("/x", "//", False),
49+
("///x/y", "/", True),
50+
("/x/y", "/", True),
51+
]
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import os
2+
from pathlib import Path
3+
4+
import pytest
5+
6+
from app.assets.helpers import path_prefix_matcher
7+
8+
from .path_prefix_cases import anchor_case_paths, prefix_case_paths
9+
10+
11+
def _is_relative_to_any(path: str, prefixes: list[str]) -> bool:
12+
candidate = Path(os.path.abspath(path))
13+
return any(candidate.is_relative_to(os.path.abspath(prefix)) for prefix in prefixes)
14+
15+
16+
def test_matches_path_is_relative_to(tmp_path):
17+
root = os.path.abspath(str(tmp_path / "root"))
18+
other = os.path.abspath(str(tmp_path / "models" / "checkpoints"))
19+
for prefixes in ([root], [other, root], [root + os.sep]):
20+
matches = path_prefix_matcher(prefixes)
21+
for path, _ in prefix_case_paths(root):
22+
assert matches(path) == _is_relative_to_any(path, prefixes), (path, prefixes)
23+
24+
25+
def test_no_prefixes_matches_nothing(tmp_path):
26+
assert path_prefix_matcher([])(str(tmp_path)) is False
27+
28+
29+
def test_filesystem_root_contains_everything(tmp_path):
30+
path = os.path.abspath(str(tmp_path))
31+
assert path_prefix_matcher([Path(path).anchor])(path) is True
32+
33+
34+
@pytest.mark.skipif(os.sep != "/", reason="POSIX anchors; Windows drives and UNC shares normalize differently")
35+
def test_matches_path_is_relative_to_across_posix_anchors():
36+
for path, prefix, expected in anchor_case_paths():
37+
assert _is_relative_to_any(path, [prefix]) is expected, (path, prefix)
38+
assert path_prefix_matcher([prefix])(path) is expected, (path, prefix)

‎tests-unit/assets_test/services/test_path_utils.py‎

Lines changed: 34 additions & 0 deletions

0 commit comments

Comments
 (0)