Measure --use-gpu on real hardware; cap the quality-pool oversubscription - #28
Merged
Merged
Conversation
…tion Pushed into GPU coverage as requested, but measured before adding anything: real head-to-head on a GTX 1650 Ti (4GB), same real 15-frame session, same machine. --use-gpu is currently a net loss -- 54.4s vs ~37s CPU-only. Root cause is mostly architectural: Phase 1's GPU-mode worker count is capped by VRAM (max_gpu_workers, ~7 workers on this card vs 16 CPU-mode workers), while most of Phase 1's actual work (lacosmic, debayer interpolation, star detection) stays CPU-native regardless of --use-gpu -- only calibration/hot-pixel/white-balance dispatch to GPU at all, so the lost worker-count parallelism isn't compensated. Found and fixed a smaller, real, secondary contributor: execute_frame_processing's GPU-mode quality-metrics thread pool ran uncapped (os.cpu_count() threads) concurrently with the GPU worker threads in the same process, and neither pool is rayon/BLAS-pinned the way CPU mode's ProcessPoolExecutor workers are (that pinning is subprocess-initializer-only). Extracted the sizing into _gpu_quality_pool_size so the two pools' thread counts stay near the physical core budget together instead of stacking. Recovered ~0.7s of the ~17s gap -- real, but confirms the worker-count architecture is the dominant cause, not thread pinning. Widening GPU coverage further (warp/drizzle) was considered and set aside: more per-worker VRAM demand would shrink the already- constraining worker count further, compounding the problem rather than fixing it. The actual fix is architectural (decouple Phase 1 worker count from VRAM when the GPU-dispatched fraction of work is small, or reduce GPU_PHASE1_WORKER_MB's footprint) and wasn't attempted here -- real OOM risk on other users' cards if the budget is cut wrong, and no evidence yet on whether it pays off on a larger card. Documented in CLAUDE.md for whoever picks this back up. New unit test for the sizing helper; full suite (1656 tests) passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Merged
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.
Summary
Pushed into GPU coverage as requested — but measured before adding anything, since this is real hardware this time (GTX 1650 Ti, 4GB), not a hypothesis.
--use-gpuis currently a net loss — 54.4s vs ~37s CPU-only.--use-gpu— only calibration/hot-pixel/white-balance dispatch to GPU at all, so the lost worker-count parallelism isn't compensated.os.cpu_count()threads) concurrently with the GPU worker threads, and neither pool is rayon/BLAS-pinned the way CPU mode's process-pool workers are. Capped it (_gpu_quality_pool_size) — recovered ~0.7s of the ~17s gap. Real, but confirms the worker-count architecture is the dominant cause, not thread pinning.Test plan
ruff check .clean🤖 Generated with Claude Code