perf(images): run image work on two own pools and trim after jobs (#523) (#582)

* perf(images): read PNG text without decoding pixels (#523)

Pillow's PngImageFile.text loads every pixel to reach text chunks written
after IDAT. The scan's metadata batch and the image monitor now read PNG
text through utils/png_text.py, which runs Pillow's own chunk handlers
after the first IDAT and seeks over the pixel data.

The legacy .text read is the floor: a differential test compares both
readers on every fixture image, a curated corpus and a seeded fuzz. The
only difference is a file whose pixel data is undecodable or cut short:
it now keeps the text read before its pixels instead of reading nothing.
Animated PNGs still use .text.

* test(images): cover the text budget, XMP and layout guard of the PNG text walk (#523)

* perf(images): run image work on two own pools and trim after jobs (#523)

Image work no longer spreads over the default executor's threads, where
glibc kept an image-sized malloc arena per thread.

- Interactive image work (view previews behind their gate, workspace card
  facts, both inspector routes) runs on the API's fixed pm-image pool of
  3 threads; the on-demand preview pool is merged into it.
- map_parallel (scan metadata, thumbnail and convert jobs, the legacy
  gallery generation, capture previews) shares one long-lived pm-worker
  pool of Worker threads, reused between jobs. The package teardown stops
  it within its bound (part "workers").
- After a scan, thumbnail or convert job, glibc's malloc_trim(0) returns
  free pages once; a no-op on Windows, macOS and musl.
- tools/perf/memory_drive.py measures RSS, heap, threads and arenas under
  the #523 load on the disposable browser test server (manual only).

* fix(images): start no image thread after the teardown; join retired job pools (#523)
This commit is contained in:
Vito
2026-11-01 01:24:27 +00:00
committed by GitHub
parent 7a2a04b752
commit 8a81114dfc
19 changed files with 1251 additions and 83 deletions
+5 -5
View File
@@ -908,20 +908,20 @@ class TestWorkspaceImages(ImageAPITestCase):
async def test_one_cards_parse_error_spares_the_page(self):
prompt = self._save_prompt("a fox")
for name, mtime in (("bad.png", 200), ("ok.png", 100)):
# Cards are read on the image pool (#523), in no fixed order: the
# parser fails on bad.png's own record (seed 6), not on a call count.
for name, mtime, seed in (("bad.png", 200, 6), ("ok.png", 100, 5)):
path = records_png(
self.output_dir / name, seeded_records("a fox", 5), mtime
self.output_dir / name, seeded_records("a fox", seed), mtime
)
self.api.db.link_image_to_prompt(prompt, str(path))
real_parse = header_params_of_images()
def parsing(info):
if parsing.calls == 0:
parsing.calls += 1
if '"seed": 6' in info["prompt"]:
raise RuntimeError("unexpected parser bug")
return real_parse(info)
parsing.calls = 0
with (
patch("py.api.images.header_params", side_effect=parsing),
self.assertLogs(self.api.logger, "WARNING") as logged,
+12
View File
@@ -146,6 +146,18 @@ class TestPreviewQueueRefusedAtShutdown(unittest.TestCase):
class TestMapParallelRefusedAtShutdown(unittest.TestCase):
def setUp(self):
# The job pool is long-lived (#523): start each test without one, so
# the patched pool class is the one created.
self.assertTrue(parallel.shutdown_job_pool(30))
self.addCleanup(parallel.shutdown_job_pool, 30)
def test_the_refused_pool_is_not_reused(self):
with mock.patch.object(parallel, "ThreadPoolExecutor", _ShutPool):
parallel.map_parallel(lambda n: n, [1, 2], 2)
self.assertFalse(parallel.job_pool_started())
self.assertEqual(parallel.map_parallel(lambda n: n + 1, [1, 2], 2), [2, 3])
def test_a_shut_pool_runs_the_items_on_the_calling_thread(self):
records = _capture(self, "prompt_manager.parallel")
with mock.patch.object(parallel, "ThreadPoolExecutor", _ShutPool):
+241
View File
@@ -0,0 +1,241 @@
"""Image work runs on PromptManager's own pools, never the default executor (#523).
glibc keeps each thread's malloc arena at its high-water mark, so image work
spread over the default executor's threads (min(32, cpus + 4)) kept an
image-sized buffer per thread. Interactive image work (card facts, inspector
reads, view previews) runs on the fixed interactive image pool
(``pm-image``); jobs (scan metadata, thumbnail jobs, capture previews) on the
long-lived Worker threads pool (``pm-worker``). After a scan or thumbnail
job, glibc's free pages are released once (``malloc_trim``).
"""
import asyncio
import threading
from unittest.mock import patch
import py.api as api_package
import py.api.images as images_module
import py.api.inspection_service as inspection_service
import py.api.thumbnail_auto as auto
from utils import parallel
try:
from tests.test_api_images import make_png
from tests.test_thumbnail_auto import WAIT, AutoThumbnailCase, _entry
except ImportError: # discovered with tests/ as the top-level directory
from test_api_images import make_png
from test_thumbnail_auto import WAIT, AutoThumbnailCase, _entry
IMAGE_THREAD = auto.POOL_PREFIX + "_"
JOB_THREAD = parallel.JOB_THREAD_PREFIX + "_"
class ImagePoolCase(AutoThumbnailCase):
async def asyncSetUp(self):
await super().asyncSetUp()
self.assertTrue(parallel.shutdown_job_pool(WAIT))
self.addCleanup(parallel.shutdown_job_pool, WAIT)
self.threads = []
self.lock = threading.Lock()
def spy(self, owner, name):
"""Record the thread every call of ``owner.name`` runs on."""
original = getattr(owner, name)
def recording(*args, **kwargs):
with self.lock:
self.threads.append(threading.current_thread().name)
return original(*args, **kwargs)
patcher = patch.object(owner, name, recording)
patcher.start()
self.addCleanup(patcher.stop)
def assert_threads(self, prefix):
self.assertTrue(self.threads, "the spied call never ran")
self.assertTrue(
all(name.startswith(prefix) for name in self.threads), self.threads
)
async def run_job(self, path, body=None):
response = await self.client.post(path, json=body or {})
self.assertEqual(response.status, 202, await response.text())
job = self.api._jobs.get((await response.json())["job"]["id"])
await asyncio.wait_for(asyncio.shield(job.task), WAIT)
return job
class TestInteractiveImagePool(ImagePoolCase):
async def test_workspace_card_facts_are_read_on_the_image_pool(self):
for index in range(5):
make_png(self.output_dir / f"c{index}.png", size=(32, 32))
self.spy(images_module, "_card_header")
response = await self.client.get(
"/prompt_manager/images/output?mode=workspace&limit=100"
)
self.assertEqual(response.status, 200, await response.text())
images = (await response.json())["images"]
self.assertEqual([image["width"] for image in images], [32] * 5)
self.assert_threads(IMAGE_THREAD)
self.assertEqual(len(self.threads), 5)
async def test_cached_card_facts_need_no_image_pool_trip(self):
make_png(self.output_dir / "c.png", size=(32, 32))
url = "/prompt_manager/images/output?mode=workspace&limit=100"
await (await self.client.get(url)).read()
self.spy(images_module, "_card_header")
response = await self.client.get(url)
self.assertEqual((await response.json())["images"][0]["width"], 32)
self.assertEqual(self.threads, [])
async def test_the_inspector_reads_on_the_image_pool(self):
make_png(self.output_dir / "i.png", size=(32, 32))
self.spy(inspection_service, "read_inspection_file")
response = await self.client.get("/prompt_manager/images/inspect/i.png?root=0")
self.assertEqual(response.status, 200, await response.text())
self.assert_threads(IMAGE_THREAD)
async def test_view_previews_and_card_reads_share_three_threads(self):
names = [f"v{i}.png" for i in range(auto.ON_DEMAND_LIMIT + 3)]
for name in names:
make_png(self.output_dir / name, size=(64, 64))
calls = []
original = self.api._generate_one
def recording(*args, **kwargs):
calls.append(threading.current_thread().name)
return original(*args, **kwargs)
self.api._generate_one = recording
for name in names:
url = _entry(self.api, self.output_dir, name)["thumbnail_url"]
await (await self.client.get(url)).read()
self.assertEqual(len(calls), len(names))
self.assertTrue(all(name.startswith(IMAGE_THREAD) for name in calls), calls)
self.assertLessEqual(len(self.api._image_pool_threads()), auto.IMAGE_POOL_SIZE)
def test_map_image_keeps_order_and_runs_inline_on_a_pool_thread(self):
self.assertEqual(self.api._map_image(lambda n: n * 2, [3, 1, 2]), [6, 2, 4])
nested = self.api._image_pool().submit(
self.api._map_image, lambda _: threading.current_thread().name, [1]
)
(name,) = nested.result(WAIT)
self.assertTrue(name.startswith(IMAGE_THREAD), name)
def test_map_image_after_a_close_runs_on_the_calling_thread(self):
pool = self.api._image_pool()
pool.shutdown(wait=True)
here = threading.current_thread().name
with patch.object(self.api, "_image_pool", return_value=pool):
names = self.api._map_image(
lambda _: threading.current_thread().name, [1, 2]
)
self.assertEqual(names, [here, here])
async def test_after_the_final_close_no_image_thread_starts(self):
await asyncio.get_running_loop().run_in_executor(
None, lambda: self.api.close_auto_thumbnails(final=True)
)
self.assertIsNone(self.api._image_pool())
here = threading.current_thread().name
names = self.api._map_image(lambda _: threading.current_thread().name, [1])
self.assertEqual(names, [here])
name = await self.api._run_image(lambda: threading.current_thread().name)
self.assertFalse(name.startswith(IMAGE_THREAD), name)
self.assertEqual(self.api._image_pool_threads(), [])
def test_map_image_runs_items_a_shutdown_cancelled_here(self):
entered = threading.Barrier(auto.IMAGE_POOL_SIZE + 1)
release = threading.Event()
self.addCleanup(release.set)
pool = self.api._image_pool()
for _ in range(auto.IMAGE_POOL_SIZE): # occupy every pool thread
pool.submit(lambda: (entered.wait(WAIT), release.wait(WAIT)))
entered.wait(WAIT)
here = threading.current_thread().name
results, submitted = {}, threading.Event()
class Counting:
calls = 0
def submit(self, *args):
future = pool.submit(*args)
Counting.calls += 1
if Counting.calls == 2:
submitted.set()
return future
def call():
with patch.object(self.api, "_image_pool", return_value=Counting()):
results["names"] = self.api._map_image(
lambda _: threading.current_thread().name, [1, 2]
)
caller = threading.Thread(target=call, name="map-image-caller")
caller.start()
# The caller's two items are queued behind the busy threads; the
# shutdown cancels them and the caller runs them itself.
self.assertTrue(submitted.wait(WAIT))
pool.shutdown(wait=False, cancel_futures=True)
caller.join(WAIT)
release.set()
self.assertFalse(caller.is_alive())
self.assertEqual(results["names"], ["map-image-caller"] * 2)
self.assertNotEqual(here, "map-image-caller")
async def test_run_image_after_a_close_uses_the_default_executor(self):
pool = self.api._image_pool()
pool.shutdown(wait=True)
with patch.object(self.api, "_image_pool", return_value=pool):
name = await self.api._run_image(lambda: threading.current_thread().name)
self.assertFalse(name.startswith(IMAGE_THREAD), name)
class TestJobPool(ImagePoolCase):
def test_scan_metadata_is_read_on_the_job_pool(self):
paths = [
make_png(self.output_dir / f"s{i}.png", size=(16, 16)) for i in range(4)
]
self.spy(self.api, "_extract_comfyui_metadata")
results = self.api._extract_batch_metadata_sync(paths)
self.assertEqual([path for path, _ in results], paths)
self.assert_threads(JOB_THREAD)
async def test_thumbnail_job_writes_on_the_job_pool_and_trims_after(self):
for index in range(3):
make_png(self.output_dir / f"t{index}.png", size=(64, 64))
self.spy(self.api, "_generate_one")
with patch.object(api_package, "release_free_memory") as trim:
job = await self.run_job("/prompt_manager/jobs/thumbnails")
self.assertEqual(job.state, "finished")
self.assert_threads(JOB_THREAD)
trim.assert_called_once_with()
async def test_scan_job_trims_after_it_finishes(self):
make_png(self.output_dir / "s.png", size=(16, 16))
states = []
def trim():
states.append(self.api._jobs.active("scan"))
return True
with patch.object(api_package, "release_free_memory", side_effect=trim):
job = await self.run_job("/prompt_manager/jobs/scan")
self.assertEqual(job.state, "finished")
self.assertGreaterEqual(len(states), 1)
self.assertIsNone(states[0], "trimmed after the scan finished")
async def test_a_failing_trim_never_fails_the_job(self):
make_png(self.output_dir / "t.png", size=(64, 64))
with patch.object(
api_package, "release_free_memory", side_effect=RuntimeError("no")
):
job = await self.run_job("/prompt_manager/jobs/thumbnails")
self.assertEqual(job.state, "finished")
async def test_capture_previews_write_on_the_job_pool(self):
path = make_png(self.output_dir / "a.png", size=(64, 64))
self.spy(self.api, "_generate_one")
self.assertEqual(self.api.queue_capture_thumbnail(str(path)), "queued")
await self._idle()
self.assert_threads(JOB_THREAD)
+39
View File
@@ -799,6 +799,45 @@ class TestPackageTeardown(PackageTeardownCase):
self.assertIsNone(seen["worker"])
self.assertTrue(all(result.values()), result)
def test_teardown_stops_the_job_pool_once_a_job_used_it(self):
"""#523: the long-lived job pool is a teardown part of its own."""
package = self.load()
parallel = sys.modules[package.__name__ + ".utils.parallel"]
self.assertEqual(parallel.map_parallel(lambda n: n * 2, [1, 2], 2), [2, 4])
# This package's own pool (the suite's top-level one may also run).
threads = list(parallel._pool[1]._threads)
self.assertTrue(threads)
result = package._stop_background_work()
self.assertEqual(list(result), ["monitor", "thumbnails", "workers", "tracker"])
self.assertTrue(all(result.values()), result)
self.assertFalse(parallel.job_pool_started())
self.assertFalse(any(t.is_alive() for t in threads))
def test_teardown_reports_a_job_thread_past_its_bound(self):
package = self.load(require_stopped=False)
parallel = sys.modules[package.__name__ + ".utils.parallel"]
entered, release = threading.Event(), threading.Event()
def hold(_):
entered.set()
release.wait(30)
caller = threading.Thread(target=parallel.map_parallel, args=(hold, [1], 1))
caller.start()
self.addCleanup(caller.join, 30)
self.addCleanup(release.set)
self.assertTrue(entered.wait(30))
with (
mock.patch.object(parallel, "JOB_POOL_JOIN_SECONDS", 0.05),
self.assertLogs("prompt_manager.init", "WARNING") as logs,
):
result = package._stop_background_work()
self.assertFalse(result["workers"])
self.assertTrue(result["tracker"])
self.assertRegex("\n".join(logs.output), r"not stopped in time .*workers")
def test_teardown_kills_ffmpeg_and_stops_the_preview_worker(self):
package = self.load()
held = self.hold_a_preview(package, advance=4.0, kill_releases=True)
+82
View File
@@ -0,0 +1,82 @@
"""``release_free_memory``: glibc's malloc_trim(0), a no-op elsewhere (#523)."""
import sys
import unittest
from unittest import mock
from utils import native_memory
class ReleaseFreeMemoryTests(unittest.TestCase):
def setUp(self):
# Each test looks the symbol up again.
patcher = mock.patch.multiple(native_memory, _trim=None, _looked_up=False)
patcher.start()
self.addCleanup(patcher.stop)
@unittest.skipUnless(native_memory._glibc(), "glibc only")
def test_glibc_trims_once_per_call(self):
self.assertTrue(native_memory.release_free_memory())
self.assertTrue(native_memory.release_free_memory())
def test_other_platforms_do_nothing(self):
for platform in ("win32", "darwin", "cygwin"):
with self.subTest(platform), mock.patch.object(sys, "platform", platform):
native_memory._looked_up = False
with mock.patch.object(native_memory.ctypes, "CDLL") as cdll:
self.assertFalse(native_memory.release_free_memory())
cdll.assert_not_called()
def test_musl_does_nothing(self):
with (
mock.patch.object(sys, "platform", "linux"),
mock.patch.object(
native_memory.os, "confstr", side_effect=ValueError("unknown name")
),
mock.patch.object(native_memory.ctypes, "CDLL") as cdll,
):
self.assertFalse(native_memory.release_free_memory())
cdll.assert_not_called()
def test_a_missing_symbol_is_false(self):
library = mock.Mock(spec=[]) # no malloc_trim attribute
with (
mock.patch.object(native_memory, "_glibc", return_value=True),
mock.patch.object(native_memory.ctypes, "CDLL", return_value=library),
):
self.assertFalse(native_memory.release_free_memory())
def test_an_unloadable_libc_is_false(self):
with (
mock.patch.object(native_memory, "_glibc", return_value=True),
mock.patch.object(native_memory.ctypes, "CDLL", side_effect=OSError),
):
self.assertFalse(native_memory.release_free_memory())
def test_a_failing_trim_never_raises(self):
trim = mock.Mock(side_effect=OSError("boom"))
library = mock.Mock(malloc_trim=trim)
with (
mock.patch.object(native_memory, "_glibc", return_value=True),
mock.patch.object(native_memory.ctypes, "CDLL", return_value=library),
):
self.assertFalse(native_memory.release_free_memory())
trim.assert_called_once_with(0)
def test_the_symbol_is_looked_up_once(self):
trim = mock.Mock(return_value=1)
library = mock.Mock(malloc_trim=trim)
with (
mock.patch.object(native_memory, "_glibc", return_value=True),
mock.patch.object(
native_memory.ctypes, "CDLL", return_value=library
) as cdll,
):
self.assertTrue(native_memory.release_free_memory())
self.assertTrue(native_memory.release_free_memory())
cdll.assert_called_once_with(None)
self.assertEqual(trim.call_count, 2)
if __name__ == "__main__":
unittest.main()
+4 -3
View File
@@ -291,7 +291,7 @@ class TestThumbnailOnFirstView(AutoThumbnailCase):
await self._writers(state, auto.ON_DEMAND_LIMIT)
await self._waiting(len(names))
self.assertEqual(default_calls, [], "no view waits on a default thread")
self.assertTrue(all(name.startswith("pm-preview") for name in state["threads"]))
self.assertTrue(all(name.startswith("pm-image") for name in state["threads"]))
status = await self.client.get("/prompt_manager/images/thumbnails/status")
self.assertEqual(status.status, 200, "another route answers meanwhile")
await status.read()
@@ -678,7 +678,7 @@ class TestShutdown(AutoThumbnailCase):
_entry(self.api, self.output_dir, "viewed.png")["thumbnail_url"]
)
await resp.read()
pool_threads = list(self.api._preview_pool_threads())
pool_threads = list(self.api._image_pool_threads())
self.assertTrue(pool_threads)
loop = asyncio.get_running_loop()
await loop.run_in_executor(
@@ -904,7 +904,8 @@ class TestThumbnailOnCapture(AutoThumbnailCase):
self.api.queue_capture_thumbnail(str(path))
release.set()
await self._idle()
self.assertEqual(batches, [(1, 1), (2, 2), (2, 2)])
# The job pool's size is the setting; each batch is at most that (#523).
self.assertEqual(batches, [(1, 2), (2, 2), (2, 2)])
self.assertLessEqual(state["peak"], 2)
for i in range(5):
self.assertTrue(
+110 -4
View File
@@ -1,4 +1,8 @@
"""Tests for utils/parallel.py: bounded thread-pool map used by long jobs."""
"""Tests for utils/parallel.py: bounded thread-pool map used by long jobs.
The jobs share one long-lived pool of Worker threads (#523): its threads (and
their malloc arenas) are reused from job to job instead of a new set per call.
"""
import os
import sys
@@ -8,10 +12,22 @@ import unittest
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
from utils.parallel import map_parallel # noqa: E402
from utils import parallel # noqa: E402
from utils.parallel import JOB_THREAD_PREFIX, map_parallel # noqa: E402
WAIT = 30
def _job_threads():
return {
t for t in threading.enumerate() if t.name.startswith(JOB_THREAD_PREFIX + "_")
}
class TestMapParallel(unittest.TestCase):
def setUp(self):
self.assertTrue(parallel.shutdown_job_pool(WAIT))
self.addCleanup(parallel.shutdown_job_pool, WAIT)
def test_results_keep_input_order(self):
def slow_square(n):
@@ -22,12 +38,102 @@ class TestMapParallel(unittest.TestCase):
map_parallel(slow_square, [1, 2, 3, 4], workers=4), [1, 4, 9, 16]
)
def test_one_worker_runs_on_the_calling_thread(self):
def test_one_worker_runs_on_the_one_job_thread(self):
names = []
map_parallel(
lambda _: names.append(threading.current_thread().name), [1, 2, 3], 1
)
self.assertEqual(set(names), {threading.current_thread().name})
self.assertEqual(len(set(names)), 1)
self.assertTrue(names[0].startswith(JOB_THREAD_PREFIX + "_"), names)
def test_jobs_reuse_the_same_long_lived_threads(self):
"""Arenas are reused instead of a new set of threads per job."""
first, second = set(), set()
map_parallel(lambda _: first.add(threading.current_thread()), range(8), 2)
map_parallel(lambda _: second.add(threading.current_thread()), range(8), 2)
self.assertTrue(second <= first, (first, second))
self.assertLessEqual(len(_job_threads()), 2)
self.assertTrue(all(t.is_alive() for t in first | second))
def test_a_new_worker_count_replaces_the_pool(self):
map_parallel(lambda n: n, range(4), 2)
old = _job_threads()
map_parallel(lambda n: n, range(4), 3)
for thread in old:
thread.join(WAIT)
self.assertFalse(thread.is_alive(), thread.name)
self.assertLessEqual(len(_job_threads()), 3)
def test_shutdown_also_joins_a_pool_a_resize_retired(self):
entered, release = threading.Event(), threading.Event()
def hold(_):
entered.set()
release.wait(WAIT)
caller = threading.Thread(target=map_parallel, args=(hold, [1], 1))
caller.start()
self.addCleanup(caller.join, WAIT)
self.addCleanup(release.set)
self.assertTrue(entered.wait(WAIT))
map_parallel(lambda n: n, [1, 2], 2) # a new size retires the busy pool
self.assertFalse(parallel.shutdown_job_pool(0.05))
release.set()
caller.join(WAIT)
self.assertTrue(parallel.shutdown_job_pool(WAIT))
self.assertFalse(parallel.job_pool_started())
def test_an_item_run_from_a_job_thread_runs_inline(self):
"""A nested map never waits for the threads it is running on."""
def outer(n):
return map_parallel(lambda m: (m, threading.current_thread()), [n], 1)
results = map_parallel(outer, [1, 2], 1)
self.assertEqual([r[0][0] for r in results], [1, 2])
def test_shutdown_stops_the_threads_and_a_later_job_starts_fresh(self):
map_parallel(lambda n: n, range(4), 2)
threads = _job_threads()
self.assertTrue(threads)
self.assertTrue(parallel.job_pool_started())
self.assertTrue(parallel.shutdown_job_pool(WAIT))
self.assertFalse(parallel.job_pool_started())
self.assertFalse(any(t.is_alive() for t in threads))
self.assertEqual(map_parallel(lambda n: n + 1, [1], 2), [2])
def test_shutdown_reports_a_thread_still_running(self):
entered, release = threading.Event(), threading.Event()
def hold(_):
entered.set()
release.wait(WAIT)
caller = threading.Thread(target=map_parallel, args=(hold, [1], 1))
caller.start()
self.assertTrue(entered.wait(WAIT))
self.assertFalse(parallel.shutdown_job_pool(0.05))
release.set()
caller.join(WAIT)
self.assertFalse(caller.is_alive())
def test_a_failing_item_waits_for_the_running_ones(self):
"""The caller sees the error only after its other items stopped."""
running = threading.Event()
finished = []
def item(n):
if n == 0:
self.assertTrue(running.wait(WAIT))
raise ValueError("zero")
running.set()
time.sleep(0.05)
finished.append(n)
return n
with self.assertRaises(ValueError):
map_parallel(item, [0, 1], 2)
self.assertEqual(finished, [1])
def test_several_workers_use_several_threads(self):
names = set()