Repository navigation
Start CPU workers safely and pass them state per call - #72
Merged
Merged
Conversation
Workers were forked (unsafe while OpenCV, BLAS or FFT threads run: a child can inherit a held lock) and read their arrays from a module-level dict, so two magnify_motions calls at once in one process would clash. Workers now start with forkserver (spawn where unavailable), with numpy, scipy, dtcwt and cv2 preloaded in the fork server. Each call allocates its arrays as anonymous in-memory files (memfd_create; temp files on other platforms) and passes only their paths; workers memory-map them. memfd avoids Docker's 64 MB /dev/shm limit. The result is returned as a view of its mapping (no copy). No module-level state remains. face.mp4, --jobs 2: ~60 s (was ~56 s), peak 2.5 GiB (was 2.3 GiB); output is bit-identical across --jobs values. Closes #69
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
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.
Closes #69
Changes
Start method: workers use
forkserver(spawnwhere it doesn't exist), never plainfork. The fork server preloads numpy, scipy.fft/ndimage, dtcwt and cv2, so workers share those pages. The CPU path now also parallelises on platforms withoutfork; the old code fell back to serial there.No module state:
_POOLis gone. Eachmagnify_motionscall allocates its input, lowpass, per-level highpass and result arrays with_SharedArrays:memfd_create), which workers open via/proc/<pid>/fd/<n>.Workers receive only paths and memory-map them.
jobs=1uses plain arrays and no pool.Why memfd and not
/dev/shmormultiprocessing.shared_memory: Docker's default/dev/shmis 64 MB. My first attempt with files there crashed workers (BrokenProcessPool) on real clips. memfd isn't limited by that mount.Pool lifetime: one pool per call, used for both the forward and inverse passes. The result is returned as an
ndarrayview of its mapping, with no copy. File descriptors are released when the call ends, and the result's mapping when it is freed._transformusesfunctools.lru_cache(per process).README notes that scripts calling
magnify_motionswithjobs> 1 need anif __name__ == "__main__":guard (standard for forkserver/spawn). Performance numbers and CHANGELOG updated.Measurements (face.mp4, default settings)
--jobs 2time--jobs 2peak (cgroup)--jobs 1time--jobs 1vs--jobs 2The cost comes from each worker's own Python heap and the fork server process.
estimate_memorystill matches (2.48 GiB estimated).Tests
fork; memfds are released (only the returned result's mapping remains, and it goes when the result is freed).