perf(images): walk the output with scandir entries and build folders from strings (#348)
The live test showed a read during a background refresh taking 2.2 s instead of 0.07 s: the walk held the GIL for about 2 s in the server. py-spy split that walk into two halves: Path.relative_to per file in folder_sets, and Path.stat + Path.is_symlink + Path joins per file in the scan. - _walk_media visits folders in os.walk's order with os.scandir entries: the file type comes from the listing (no lstat per file) and the mtime from entry.stat (one call, none on Windows). _iter_media_files is now a thin wrapper, so its other caller is unchanged. - folder_sets slices the path strings (in the path flavour's separator) instead of calling relative_to, falling back to it for a path that does not start with the root string. On the maintainer's 19,935-file output folder: scan 0.386 s -> 0.112 s, folder sets 0.452 s -> 0.007 s, both results identical to the previous code. A reference copy of the os.walk version checks order, filters, the depth cap, symlinks and the file cap.
This commit is contained in:
@@ -91,9 +91,19 @@ def folder_sets(root, files) -> tuple[frozenset, frozenset]:
|
||||
``str(relative.parent)`` (native separators) for the folders holding
|
||||
files, plus every ancestor joined by ``/``.
|
||||
"""
|
||||
# From the path strings: relative_to per file was half of a large walk.
|
||||
# The separator is the path flavour's own (tests use PureWindowsPath).
|
||||
sep = str(type(root)("a", "b"))[1]
|
||||
prefix = str(root)
|
||||
if not prefix.endswith(sep):
|
||||
prefix += sep
|
||||
folders = set()
|
||||
for path, _mtime in files:
|
||||
rel_dir = str(path.relative_to(root).parent)
|
||||
text = str(path)
|
||||
if text.startswith(prefix):
|
||||
rel_dir = text[len(prefix) :].rpartition(sep)[0]
|
||||
else:
|
||||
rel_dir = str(path.relative_to(root).parent)
|
||||
if rel_dir and rel_dir != ".":
|
||||
folders.add(rel_dir)
|
||||
with_ancestors = set(folders)
|
||||
|
||||
+42
-17
@@ -772,32 +772,54 @@ def _favorite_root_key(root) -> str:
|
||||
return _favorite_relative_key(os.path.realpath(str(root)))
|
||||
|
||||
|
||||
def _iter_media_files(output_path, max_files):
|
||||
"""Yield media files under *output_path* with bounded effort.
|
||||
def _walk_media(output_path, max_files):
|
||||
"""Yield the ``os.DirEntry`` of each media file under *output_path*.
|
||||
|
||||
Symlinks are never followed, ``thumbnails/`` is skipped, descent stops
|
||||
at MAX_SCAN_DEPTH and at most *max_files* paths are produced.
|
||||
at MAX_SCAN_DEPTH and at most *max_files* entries are produced. The order
|
||||
is ``os.walk``'s (top-down, subfolders in ``os.scandir`` order, files by
|
||||
name), which the gallery's ties in mtime keep. Each entry's type comes
|
||||
from the directory listing itself, so a walk builds no ``Path`` and makes
|
||||
no extra ``lstat`` per file (#348: a 20,000-file walk spent half its time
|
||||
there).
|
||||
"""
|
||||
output_path = Path(output_path)
|
||||
root_depth = len(output_path.parts)
|
||||
stack = [(os.fspath(output_path), ())]
|
||||
produced = 0
|
||||
for root, dirs, files in os.walk(output_path):
|
||||
root_path = Path(root)
|
||||
rel_parts = root_path.parts[root_depth:]
|
||||
while stack:
|
||||
top, rel_parts = stack.pop()
|
||||
if "thumbnails" in rel_parts:
|
||||
dirs[:] = []
|
||||
continue
|
||||
if len(rel_parts) >= MAX_SCAN_DEPTH:
|
||||
dirs[:] = []
|
||||
for name in sorted(files):
|
||||
if Path(name).suffix.lower() not in MEDIA_EXTENSIONS:
|
||||
dirs, files = [], []
|
||||
try:
|
||||
with os.scandir(top) as entries:
|
||||
for entry in entries:
|
||||
try:
|
||||
is_dir = entry.is_dir()
|
||||
except OSError:
|
||||
is_dir = False
|
||||
(dirs if is_dir else files).append(entry)
|
||||
except OSError:
|
||||
continue # as os.walk: an unreadable folder is skipped
|
||||
for entry in sorted(files, key=lambda item: item.name):
|
||||
if os.path.splitext(entry.name)[1].lower() not in MEDIA_EXTENSIONS:
|
||||
continue
|
||||
if (root_path / name).is_symlink():
|
||||
if entry.is_symlink():
|
||||
continue # never read through a link out of the output tree
|
||||
if produced >= max_files:
|
||||
return
|
||||
produced += 1
|
||||
yield root_path / name
|
||||
yield entry
|
||||
if len(rel_parts) >= MAX_SCAN_DEPTH:
|
||||
continue
|
||||
for entry in reversed(dirs):
|
||||
if not entry.is_symlink():
|
||||
stack.append((entry.path, rel_parts + (entry.name,)))
|
||||
|
||||
|
||||
def _iter_media_files(output_path, max_files):
|
||||
"""Yield the ``Path`` of each media file under *output_path* (see _walk_media)."""
|
||||
for entry in _walk_media(output_path, max_files):
|
||||
yield Path(entry.path)
|
||||
|
||||
|
||||
def media_thumbnail_rel(rel_path, settings=None):
|
||||
@@ -1294,11 +1316,14 @@ class ImageRoutesMixin:
|
||||
walk is bounded by MAX_SCAN_DEPTH and MAX_GALLERY_FILES.
|
||||
"""
|
||||
found = []
|
||||
for media_path in _iter_media_files(output_path, MAX_GALLERY_FILES):
|
||||
for entry in _walk_media(output_path, MAX_GALLERY_FILES):
|
||||
try:
|
||||
found.append((media_path, media_path.stat().st_mtime))
|
||||
# Never a symlink (_walk_media skips them), so this is the
|
||||
# file's own mtime, read from the listing where it can be.
|
||||
mtime = entry.stat(follow_symlinks=False).st_mtime
|
||||
except OSError:
|
||||
continue
|
||||
found.append((Path(entry.path), mtime))
|
||||
if len(found) >= MAX_GALLERY_FILES:
|
||||
self.logger.warning(f"Gallery scan capped at {MAX_GALLERY_FILES} files")
|
||||
found.sort(key=lambda x: x[1], reverse=True)
|
||||
|
||||
@@ -1455,12 +1455,15 @@ class TestGalleryListing(ImageRouteCoverageCase):
|
||||
self.assertEqual(data["images"], [])
|
||||
|
||||
def test_gallery_scan_skips_unreadable_entries(self):
|
||||
ghost = self.output_dir / "ghost.png"
|
||||
make_png(self.output_dir / "real.png")
|
||||
listed = list(images_module._walk_media(self.output_dir, 10))
|
||||
ghost = MagicMock(path=str(self.output_dir / "ghost.png"))
|
||||
ghost.stat.side_effect = FileNotFoundError("gone")
|
||||
with patch.object(
|
||||
images_module, "_iter_media_files", return_value=iter([ghost])
|
||||
images_module, "_walk_media", return_value=iter([ghost, *listed])
|
||||
):
|
||||
found = self.api._scan_gallery_files_sync(self.output_dir)
|
||||
self.assertEqual(found, [])
|
||||
self.assertEqual([path.name for path, _mtime in found], ["real.png"])
|
||||
|
||||
|
||||
class TestThumbnailUnits(ImageRouteCoverageCase):
|
||||
@@ -2033,6 +2036,54 @@ class TestOutputScanHelpers(ImageAPITestCase):
|
||||
expected = hashlib.sha1(str(media).encode("utf-8")).hexdigest()[:16]
|
||||
self.assertEqual(entry["id"], expected)
|
||||
|
||||
def test_media_walk_matches_the_os_walk_reference(self):
|
||||
# The scandir walk (#348) must list exactly what the os.walk version
|
||||
# listed, in the same order: ties in mtime keep this order.
|
||||
output = Path(self.output_dir)
|
||||
for rel in (
|
||||
"z.png",
|
||||
"a.PNG",
|
||||
"notes.txt",
|
||||
"b/2.png",
|
||||
"b/1.webp",
|
||||
"b/c/deep.mp4",
|
||||
"b/c/d/too_deep.png",
|
||||
"thumbnails/t.png",
|
||||
"b/thumbnails/u.png",
|
||||
"e/.hidden.png",
|
||||
"e/f.jpeg",
|
||||
):
|
||||
make_png(output / rel)
|
||||
try:
|
||||
os.symlink(output / "z.png", output / "b" / "link.png")
|
||||
os.symlink(output / "b", output / "dirlink", target_is_directory=True)
|
||||
except (OSError, NotImplementedError):
|
||||
pass # the rest of the tree still checks order and filters
|
||||
with patch.object(images_module, "MAX_SCAN_DEPTH", 2):
|
||||
listed = list(images_module._iter_media_files(output, 100))
|
||||
expected = list(_reference_iter_media_files(output, 100))
|
||||
capped = list(images_module._iter_media_files(output, 3))
|
||||
self.assertEqual(listed, expected)
|
||||
self.assertEqual(capped, expected[:3])
|
||||
self.assertTrue(all(isinstance(path, Path) for path in listed))
|
||||
|
||||
def test_gallery_scan_reads_each_file_type_and_mtime_from_scandir(self):
|
||||
# Path.stat and Path.is_symlink per file were the other half of the
|
||||
# live walk's cost (#348); the scandir entries already carry both.
|
||||
output = Path(self.output_dir)
|
||||
make_png(output / "a.png")
|
||||
make_png(output / "sub" / "b.png")
|
||||
with (
|
||||
patch.object(Path, "stat", side_effect=AssertionError),
|
||||
patch.object(Path, "is_symlink", side_effect=AssertionError),
|
||||
):
|
||||
found = self.api._scan_gallery_files_sync(output)
|
||||
self.assertEqual(
|
||||
sorted(path.name for path, _mtime in found), ["a.png", "b.png"]
|
||||
)
|
||||
for path, mtime in found:
|
||||
self.assertEqual(mtime, os.stat(path).st_mtime)
|
||||
|
||||
def test_iter_media_files_skips_symlinked_files(self):
|
||||
output = Path(self.output_dir)
|
||||
make_png(output / "real.png")
|
||||
@@ -2355,6 +2406,30 @@ class TestRootScopeServing(ImageAPITestCase):
|
||||
_HANG_LIMIT = 30
|
||||
|
||||
|
||||
def _reference_iter_media_files(output_path, max_files):
|
||||
"""``_iter_media_files`` as it was before the scandir walk (#348)."""
|
||||
output_path = Path(output_path)
|
||||
root_depth = len(output_path.parts)
|
||||
produced = 0
|
||||
for root, dirs, files in os.walk(output_path):
|
||||
root_path = Path(root)
|
||||
rel_parts = root_path.parts[root_depth:]
|
||||
if "thumbnails" in rel_parts:
|
||||
dirs[:] = []
|
||||
continue
|
||||
if len(rel_parts) >= images_module.MAX_SCAN_DEPTH:
|
||||
dirs[:] = []
|
||||
for name in sorted(files):
|
||||
if Path(name).suffix.lower() not in images_module.MEDIA_EXTENSIONS:
|
||||
continue
|
||||
if (root_path / name).is_symlink():
|
||||
continue
|
||||
if produced >= max_files:
|
||||
return
|
||||
produced += 1
|
||||
yield root_path / name
|
||||
|
||||
|
||||
class _GatedScan:
|
||||
"""Wraps the real scan, counts calls and can hold one call after it listed.
|
||||
|
||||
|
||||
@@ -4,7 +4,8 @@ import os
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path, PureWindowsPath
|
||||
from pathlib import Path, PurePath, PureWindowsPath
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||
|
||||
@@ -70,6 +71,23 @@ class FolderSetsTests(unittest.TestCase):
|
||||
def test_an_empty_listing_has_no_folders(self):
|
||||
self.assertEqual(folder_sets(Path("/out"), []), (frozenset(), frozenset()))
|
||||
|
||||
def test_folders_come_from_the_path_strings_not_relative_to(self):
|
||||
# relative_to per file was half of a 20,000-file walk (#348 live test).
|
||||
root = Path("/out")
|
||||
files = [(root / "a" / "b" / "one.png", 1.0), (root / "two.png", 2.0)]
|
||||
with patch.object(PurePath, "relative_to", side_effect=AssertionError):
|
||||
folders, with_ancestors = folder_sets(root, files)
|
||||
self.assertEqual(folders, {os.path.join("a", "b")})
|
||||
self.assertEqual(with_ancestors, {os.path.join("a", "b"), "a"})
|
||||
|
||||
def test_a_file_outside_the_root_string_still_gets_its_folder(self):
|
||||
root = Path("/out")
|
||||
odd = Path("/out/./x/y.png") # normalizes to /out/x/y.png
|
||||
files = [(odd, 1.0)]
|
||||
self.assertEqual(
|
||||
folder_sets(root, files)[0], _legacy_subfolders(root, files, False)
|
||||
)
|
||||
|
||||
|
||||
class SameListingTests(unittest.TestCase):
|
||||
def test_equal_path_and_mtime_lists_are_the_same(self):
|
||||
|
||||
Reference in New Issue
Block a user