perf(photos): stream album enumeration in fixed-size chunks (bounds peak RSS on large libraries)#472
Open
epheterson wants to merge 2 commits into
Open
perf(photos): stream album enumeration in fixed-size chunks (bounds peak RSS on large libraries)#472epheterson wants to merge 2 commits into
epheterson wants to merge 2 commits into
Conversation
Mandarons currently materializes the entire per-photo download-task
list in memory before passing any of it to execute_parallel_downloads.
For Eric's 111K-photo iCloud library this peaks at ~4 GB RSS
during enumeration (kernel-confirmed cgroup OOM at the 4 GB cap:
total-vm:4270872kB anon-rss:4181524kB), and forces operators to size
the container at 8 GB+ even though steady-state usage is <1 GB.
Fix: replace the build-list-then-download flow with a buffer-and-drain
pipeline. _collect_and_execute_album_in_chunks() accumulates up to
chunk_size DownloadTaskInfo entries, drains them via
execute_parallel_downloads, clears the buffer, then collects the next
chunk. Memory bounded by chunk_size, not len(album).
Concrete changes:
- album_sync_orchestrator.py: new _collect_and_execute_album_in_chunks
with DEFAULT_ENUMERATION_CHUNK_SIZE=1000. Keeps the legacy
_collect_album_download_tasks as a thin wrapper for tests that call
it directly.
- config_parser.py: get_photos_enumeration_chunk_size() exposes the
knob via photos.enumeration_chunk_size in config.yaml. Defensive
about non-int / non-positive values (fall back to default rather
than crash).
- tests/test_streaming_enumeration.py: 11 new tests.
* 5 chunking equivalence + buffer-bound contract
(test_buffer_never_exceeds_chunk_size is the headline — proves
streaming is actually bounded, fires immediately if the refactor
regresses to monolithic enumeration).
* 6 config-getter cases (default, explicit int, 0, negative, garbage
string, None config).
Empirical validation pending after deploy: on Eric's library, the
streaming fix should let mem_limit drop back from 8 GB to ~2 GB. The
icloud-docker-plus repo has the validation step in
docs/plans/2026-05-29-pr12-streaming-photo-enumeration.md.
Co-Authored-By: Claude <noreply@anthropic.com>
Upstream CI runs ruff. No semantic change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
sync_album_photosbuilds the full download-task list for an album before downloading any of it, so peak resident memory grows withlen(album). On large libraries this OOM-kills the container: a ~111K-photo library peaks around 4 GB RSS and gets cgroup-OOM-killed at a 4 GB cap. The only mitigation today is over-provisioningmem_limit.Change
Stream each album in fixed-size chunks: collect up to
chunk_sizedownload tasks → drain them through the existingexecute_parallel_downloads→ release the buffer → collect the next chunk. Memory is bounded bychunk_size, notlen(album).photos.enumeration_chunk_size(default 1000; non-positive / garbage values fall back to the default with an INFO log rather than refusing to sync).filesset), same subalbum recursion — only the resident set stays flat instead of growing monotonically through enumeration.for photo in album, so it adds no new icloudpy version requirement. (Complementsiter_chunksfrom icloudpy#140 but does not depend on it.)Validation
tests/test_streaming_enumeration.py. The headlinetest_buffer_never_exceeds_chunk_sizeasserts the bound actually holds — it fires immediately if the refactor ever regresses to monolithic enumeration — plus chunking-equivalence cases and the config-getter edge cases (default, explicit int, zero, negative, garbage string, None config). 100% coverage maintained.chunk_size=1000sustains < 1 GB resident through a full enumeration (vs ~4 GB OOM before).Supersedes #462 (rebased clean onto current
main). Part of the RFC in #454.