Skip to content

Cover daemon perf: plan - #621

Merged
ajslater merged 1 commit into
v1.11-performancefrom
coverd-perf-plan
Apr 28, 2026
Merged

ajslater merged 1 commit into
v1.11-performancefrom
coverd-perf-plan

Conversation

@ajslater

Copy link
Copy Markdown
Owner

Summary

Plan for codex/librarian/covers/. Single-file plan because the
surface is small (6 files, ~440 LOC) — no meta split needed.

Pure planning deliverable, no code changes.

Eleven ranked findings

# Finding Severity
F1 Pre-fetch DB paths in the parent (N serial SELECTs → 1) refactor
F2 Skip-existing pre-filter in the parent small win
F3 ProcessPoolExecutor for the image pipeline HIGH
F4 Lazy + persistent pool lifecycle bundles with F3
F5 CODEX_COVER_WORKERS setting bundles with F3
F6 Single-pk inline fast path speculative — defer
F7 purge_cover_paths parallelization skip
F8 Stale create_cover_from_path docstring cleanup
F9 Empty @dataclass decorators on cover tasks cosmetic
F10 Status update batching speculative — defer
F11 Memory ceiling guidance documentation

Headline finding

F3 — the cover thread today serializes a CPU-bound image
pipeline through a single worker:

for pk in pks:
    self._bulk_create_comic_cover(pk, status, custom=custom)

Each iteration runs comicbox.get_cover_page + PIL
Image.thumbnail(LANCZOS, reducing_gap=3.0) + save("WEBP", method=6). On a multi-core box, 7 cores sit idle.

Plan recommends concurrent.futures.ProcessPoolExecutor (not
threads — PIL releases the GIL inconsistently across encoder
paths). Worker function is top-level (picklable), accepts
pre-resolved (pk, db_path, cover_path_str, custom) tuples,
returns (pk, thumb_bytes, error_msg). Parent uses
as_completed to stream results, writes to disk via the
existing atomic-replace save_cover_to_cache, updates status
per completion.

F1 (pre-fetch DB paths) and F2 (skip-existing pre-filter) are
called out separately because they need to land first — F3's
worker subprocess shouldn't carry a Django connection, and the
batched path fetch + skip-existing filter remove that
dependency.

Suggested ordering

  1. F1 + F2 — pure refactor, no thread changes. Land first;
    F3 builds on it.
  2. F8 + F9 — cleanups. Bundle anywhere.
  3. F3 + F4 + F5 — the big change. Process pool, lazy
    creation, settings knob. Single PR; the unit of risk is the
    subprocess introduction.

Risks flagged

  • comicbox + PIL subprocess safety (expected fine, verify via
    smoke test against a populated dev DB).
  • COMICBOX_CONFIG import cost in workers — pass the dict as a
    worker arg vs. re-importing Django settings per subprocess.
  • Loguru cross-process logging — workers return error strings
    for the parent to log, sidesteps the issue.
  • Pool shutdown on hard kill — ProcessPoolExecutor atexit
    handler should cover it; signal handler if not.
  • RAM ceiling: ~100 MB/worker × 8 workers ≈ 800 MB peak.
    Documented; honor CODEX_COVER_WORKERS for memory-tight
    installs.

References

  • Plan: tasks/coverd-perf/00-plan.md
  • codex/librarian/scribe/importer/create/comics.py:173 — main
    CoverCreateTask producer (post-import batches)
  • codex/views/browser/cover.py:104 — the 202-poll
    per-request producer (single pk per task)

🤖 Generated with Claude Code

Single-file plan because the surface is small (codex/librarian/
covers/ — 6 files, ~440 LOC). The dominant finding is F3:
parallelize the image pipeline via concurrent.futures.
ProcessPoolExecutor.

Eleven ranked findings:

- F1: Pre-fetch DB paths in the parent. Drops N serial SELECTs
  to 1, and removes Django from the worker subprocess in F3.
- F2: Skip-existing pre-filter in the parent. Drops dispatch
  overhead for already-cached covers.
- F3: ProcessPoolExecutor for the image pipeline. Workers
  return bytes; parent writes to disk + updates status via
  as_completed. Headline win — embarrassingly parallel
  workload across CPU cores, today serial on one thread.
- F4: Lazy + persistent pool lifecycle. Cold-start cost
  amortized across batches; one pool per cover-thread instance.
- F5: CODEX_COVER_WORKERS setting. Default
  min(os.cpu_count(), 8); env var override for memory-tight
  installs.
- F6: Single-pk inline fast path — speculative, defer.
- F7: purge_cover_paths parallelization — skip (unlinks are
  cheap).
- F8: Stale "Called from views/cover" docstring on
  create_cover_from_path. Cleanup.
- F9: Empty @DataClass decorators on CoverTask /
  CoverRemoveAllTask / CoverRemoveOrphansTask /
  CoverCreateAllTask. Cosmetic.
- F10: Status update batching — speculative, defer.
- F11: Memory ceiling guidance. ~100 MB per worker × 8 = 800 MB
  worst case. Documented.

Suggested ordering: F1 + F2 (pure refactor, no thread changes)
-> F8 + F9 (cleanup) -> F3 + F4 + F5 (the big change). Pre-
work first so F3 lands with the smallest unit-of-risk diff.

Risks called out: comicbox + PIL subprocess safety (unverified
but expected fine), COMICBOX_CONFIG import cost in workers,
loguru cross-process logging, pool shutdown on hard kill, RAM
ceiling under heavy worker count.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ajslater
ajslater merged commit 0c66650 into v1.11-performance Apr 28, 2026
1 check failed
@ajslater
ajslater deleted the coverd-perf-plan branch May 2, 2026 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant