From 300ea066a4a9b7dd2ce55296010705ca81e5a2b0 Mon Sep 17 00:00:00 2001 From: Alex Liberzon Date: Thu, 17 Sep 2026 17:02:55 +0300 Subject: [PATCH 1/4] perf(correspondences): replace np.searchsorted with typed bisect in hot loop _build_adjacency_for_pair (correspondences.py) called np.searchsorted once per source target inside an already-Cython-compiled loop, thousands of times per frame. Profiling a real 25-frame/4-camera batch run (py-spy) showed this cost 5.94s self-time / 6.09s cumulative -- ~13% of the 47.65s run -- purely from numpy's per-call Python/ufunc dispatch overhead, not from the search itself. Replaced with _bisect_left, a small typed C loop over the same sorted memoryview: same algorithm, same result, no numeric change. Re-profiling after the fix shows the numpy dispatch entry gone entirely from the self-time ranking. Verified against the full test suite (1924 tests): 1820 passed, 4 failed -- all four are a pre-existing ModuleNotFoundError for the optional 'flowtracks' dependency (unrelated to this change; reproduced identically on unmodified correspondences.py in this venv). Also attempted replacing match_pairs' per-frame ThreadPoolExecutor creation (~22% of wall time in the same profile) with a shared, process-wide pool. That change was reverted: a full-suite run appeared to hang for a long time at test_parallel_tracking.py. Investigation found that wall-clock measurement in this session was unreliable (a later "stuck" run turned out to complete the full suite in ~7.5 minutes), so the hang was not conclusively confirmed as a real deadlock -- but without a clean re-verification, reverting was the safer call. Left as a candidate for a future, more carefully isolated attempt. --- src/openptv2/algorithms/correspondences.py | 34 +++++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/openptv2/algorithms/correspondences.py b/src/openptv2/algorithms/correspondences.py index 021a809e..0d2e8b88 100644 --- a/src/openptv2/algorithms/correspondences.py +++ b/src/openptv2/algorithms/correspondences.py @@ -20,6 +20,30 @@ PT_UNUSED = -999 +@cython.cfunc +@cython.nogil +@cython.boundscheck(False) +@cython.wraparound(False) +def _bisect_left(arr: cython.double[:], n: cython.int, value: cython.double) -> cython.int: + """Index of the first element >= ``value`` in an ascending array of length ``n``. + + Same result as ``int(np.searchsorted(arr, value))``, but as a plain C loop + instead of numpy's per-call Python/ufunc dispatch. Called once per source + target inside ``_build_adjacency_for_pair``'s hot loop -- thousands of + times per frame -- where the numpy call's own dispatch overhead dominated. + """ + lo: cython.int = 0 + hi: cython.int = n + mid: cython.int + while lo < hi: + mid = (lo + hi) // 2 + if arr[mid] < value: + lo = mid + 1 + else: + hi = mid + return lo + + # --------------------------------------------------------------------------- # Output data type — NTupel is the external result of the matching pipeline. # It is only created / consumed once per frame (not in hot loops), so the @@ -146,9 +170,11 @@ def _build_adjacency_for_pair( Speedups applied: * :func:`epi_mm_batch` computes all N epipolar bounding boxes in one vectorised call (replacing N individual :func:`epi_mm` calls). - * :func:`numpy.searchsorted` on the x-sorted destination array finds - the epipolar-band start in O(log M) instead of the manual bisection - previously inside :func:`find_candidate`. + * :func:`_bisect_left`, a typed C loop, finds the epipolar-band start on + the x-sorted destination array in O(log M) -- replacing both the manual + bisection previously inside :func:`find_candidate` and (later) + :func:`numpy.searchsorted`, whose per-call Python/ufunc dispatch + dominated this loop when profiled. * Quality-ratio checks and distance filtering are applied inline, avoiding Python function-call overhead for :func:`find_candidate`. """ @@ -267,7 +293,7 @@ def _build_adjacency_for_pair( sqrt_m2_1: cython.double = np.sqrt(m_line * m_line + 1.0) # Binary-search for x-range start (replaces manual bisection) - lo = int(np.searchsorted(dst_x, xa - eps)) + lo = _bisect_left(dst_x, n2, xa - eps) n_i: cython.double = frm.targets[i1][src_pnr[i]].n nx_i: cython.double = frm.targets[i1][src_pnr[i]].nx From 25dfff831760bab0a7353ca472998bab39466fe9 Mon Sep 17 00:00:00 2001 From: Alex Liberzon Date: Thu, 17 Sep 2026 17:20:19 +0300 Subject: [PATCH 2/4] build: resolve flowtracks to the local sibling postptv/ checkout flowtracks is published on PyPI, but its canonical source in this workspace is the sibling postptv/ repo (top-level CLAUDE.md: "published name differs from repo name"). Without a uv.sources override, `uv sync --extra dev` had nothing telling it to prefer the local checkout, and in this venv flowtracks ended up simply absent -- causing 4 tests (test_run_store.py x3, test_zarr_store.py::test_read_zarr_trajectories) to fail with ModuleNotFoundError. Added the same [tool.uv.sources] override flowtracks_examples/pyproject.toml already uses (flowtracks = { path = "../postptv", editable = true }), then `uv sync --extra dev`. Full suite now: 1826 passed, 0 failed (up from 1820 passed / 4 failed). --- pyproject.toml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 6312bee5..620b6211 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -121,6 +121,13 @@ pyptv = "openptv2.gui.pyptv_gui:main" pyptv_gui = "openptv2.gui.pyptv_gui:main" pyptv_batch = "openptv2.batch.pyptv_batch:main_cli" +[tool.uv.sources] +# Published as "flowtracks" on PyPI, but the repo lives as sibling `postptv/` +# in this workspace (see top-level CLAUDE.md) -- same override +# flowtracks_examples/pyproject.toml uses, so a local postptv checkout is +# what `uv sync --extra gui` (or `--extra dev`) actually installs. +flowtracks = { path = "../postptv", editable = true } + [tool.setuptools] packages = [ "openptv2", From 787b209acd3aff24dfd16a2e937563101827163c Mon Sep 17 00:00:00 2001 From: Alex Liberzon Date: Thu, 17 Sep 2026 17:38:35 +0300 Subject: [PATCH 3/4] perf(correspondences): reuse a shared thread pool instead of one per frame match_pairs opened a fresh ThreadPoolExecutor on every frame to parallelize per-camera-pair adjacency search. Profiling a real 25-frame/4-camera batch run showed this cost ~22% of wall time in pure thread create/teardown overhead, not in the work itself -- confirmed by ~29 distinct ThreadPoolExecutor instances spun up over the run. Added openptv2/thread_pool.py: one process-wide executor, created on first use and reused for the life of the process. match_pairs now submits to it instead of opening its own pool per call. This fix was attempted once before and reverted: a full-suite test run appeared to hang for a long time at test_parallel_tracking.py. That investigation is now closed out as a false alarm -- the session's own elapsed-time tracking (via a scheduler that didn't correspond to real wall-clock time) was unreliable, not the fix itself. Re-verified this time with trustworthy measurements: - A foreground, `time`-wrapped run of the exact test files involved (test_parallel_correspondences.py, test_parallel_mmlut.py, test_parallel_preprocessing.py, test_parallel_tracking.py): 114.27s, matching the no-fix baseline of ~105-114s. No hang, no slowdown. - The full suite (1926 tests, accurate Get-Date timestamps throughout): 1826 passed, 0 failed, 459.20s -- same result as without this fix. No deadlock risk: the shared pool's tasks (_build_adjacency_for_pair) are leaf work with no callback into the pool, so there's no path for a worker to block waiting on another task queued behind it. --- src/openptv2/algorithms/correspondences.py | 52 ++++++++++++---------- src/openptv2/thread_pool.py | 23 ++++++++++ 2 files changed, 52 insertions(+), 23 deletions(-) create mode 100644 src/openptv2/thread_pool.py diff --git a/src/openptv2/algorithms/correspondences.py b/src/openptv2/algorithms/correspondences.py index 0d2e8b88..1d07d25a 100644 --- a/src/openptv2/algorithms/correspondences.py +++ b/src/openptv2/algorithms/correspondences.py @@ -387,29 +387,35 @@ def match_pairs( ) return - # Multi-threaded: each camera pair is independent - from concurrent.futures import ThreadPoolExecutor, as_completed - - with ThreadPoolExecutor(max_workers=len(pairs)) as pool: - futures = { - pool.submit( - _build_adjacency_for_pair, - i1, - i2, - n_arr, - p2_arr, - corr_arr, - dist_arr, - corrected, - frm, - vpar, - cpar, - calib, - ): (i1, i2) - for i1, i2 in pairs - } - for future in as_completed(futures): - future.result() # propagate exceptions + # Multi-threaded: each camera pair is independent. Uses the shared, + # process-wide pool (openptv2.thread_pool) instead of opening a fresh + # ThreadPoolExecutor here -- this runs once per frame, and a fresh pool + # per frame measured at ~22% of wall time in a profiled batch run purely + # in thread create/teardown, not in the work itself. + from concurrent.futures import as_completed + + from openptv2.thread_pool import get_executor + + pool = get_executor() + futures = { + pool.submit( + _build_adjacency_for_pair, + i1, + i2, + n_arr, + p2_arr, + corr_arr, + dist_arr, + corrected, + frm, + vpar, + cpar, + calib, + ): (i1, i2) + for i1, i2 in pairs + } + for future in as_completed(futures): + future.result() # propagate exceptions # --------------------------------------------------------------------------- diff --git a/src/openptv2/thread_pool.py b/src/openptv2/thread_pool.py new file mode 100644 index 00000000..e465c9bf --- /dev/null +++ b/src/openptv2/thread_pool.py @@ -0,0 +1,23 @@ +"""A single reusable thread pool, shared across per-frame calls. + +``match_pairs`` used to open a fresh ``ThreadPoolExecutor`` (and tear it +down) on every frame -- real OS thread creation/teardown 25+ times per batch +run, measured at ~22% of wall time in a profiled run. One pool, sized to the +machine and created once, removes that churn: submitting work to it costs +only a queue push. +""" + +import atexit +import os +from concurrent.futures import ThreadPoolExecutor + +_executor: ThreadPoolExecutor | None = None + + +def get_executor() -> ThreadPoolExecutor: + """The process-wide shared executor, created on first use.""" + global _executor + if _executor is None: + _executor = ThreadPoolExecutor(max_workers=os.cpu_count() or 4) + atexit.register(_executor.shutdown, wait=False, cancel_futures=True) + return _executor From b1811aff0aa1adb49074a72da33434eab70bbf46 Mon Sep 17 00:00:00 2001 From: Alex Liberzon Date: Thu, 17 Sep 2026 18:50:13 +0300 Subject: [PATCH 4/4] fix: address Copilot review on PR #37 - pyproject.toml: remove the [tool.uv.sources] override that force-pointed flowtracks at a sibling ../postptv path. That path only exists in this dev workspace, so it broke `uv sync` on every CI runner and standalone clone ("Distribution not found at: file:///.../postptv") -- this was the actual cause of all 4 failing CI checks. Turns out the override was never needed: plain PyPI flowtracks (1.2.2) installs fine via `uv sync --extra dev` and the previously-failing flowtracks-dependent tests pass against it -- that combination just hadn't been tested in isolation before. - correspondences.py (match_pairs): on a future exception, cancel the remaining futures and wait for them before re-raising, instead of returning immediately. The shared pool (unlike the removed `with ThreadPoolExecutor() as pool:`) outlives this call and won't wait for stragglers on its own, so an exception could previously leave other futures still running and writing into the frame's shared adjacency buffers after the caller had already moved on -- a race if the buffers are reused or the call retried. - thread_pool.py: recreate the executor when the current PID no longer matches the one that created it. pyptv_batch_parallel forks worker processes on Unix; a fork only clones the calling thread, so a child inheriting an already-initialized executor would hold a pool object whose worker threads don't exist in that process, and submissions there would hang forever with no consumer. Verified: full test suite 1826 passed, 0 failed (unchanged from before these fixes); targeted correspondences/parallel-* tests (78) also pass. --- pyproject.toml | 7 ------- src/openptv2/algorithms/correspondences.py | 18 +++++++++++++++--- src/openptv2/thread_pool.py | 17 ++++++++++++++--- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 620b6211..6312bee5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -121,13 +121,6 @@ pyptv = "openptv2.gui.pyptv_gui:main" pyptv_gui = "openptv2.gui.pyptv_gui:main" pyptv_batch = "openptv2.batch.pyptv_batch:main_cli" -[tool.uv.sources] -# Published as "flowtracks" on PyPI, but the repo lives as sibling `postptv/` -# in this workspace (see top-level CLAUDE.md) -- same override -# flowtracks_examples/pyproject.toml uses, so a local postptv checkout is -# what `uv sync --extra gui` (or `--extra dev`) actually installs. -flowtracks = { path = "../postptv", editable = true } - [tool.setuptools] packages = [ "openptv2", diff --git a/src/openptv2/algorithms/correspondences.py b/src/openptv2/algorithms/correspondences.py index 1d07d25a..1511c67f 100644 --- a/src/openptv2/algorithms/correspondences.py +++ b/src/openptv2/algorithms/correspondences.py @@ -392,7 +392,7 @@ def match_pairs( # ThreadPoolExecutor here -- this runs once per frame, and a fresh pool # per frame measured at ~22% of wall time in a profiled batch run purely # in thread create/teardown, not in the work itself. - from concurrent.futures import as_completed + from concurrent.futures import as_completed, wait from openptv2.thread_pool import get_executor @@ -414,8 +414,20 @@ def match_pairs( ): (i1, i2) for i1, i2 in pairs } - for future in as_completed(futures): - future.result() # propagate exceptions + try: + for future in as_completed(futures): + future.result() # propagate exceptions + except BaseException: + # Unlike the removed `with ThreadPoolExecutor() as pool:`, this pool + # is shared and outlives this call, so it won't wait for stragglers + # on its own. Cancel what hasn't started and wait for what has, + # so nothing keeps writing into n_arr/p2_arr/corr_arr/dist_arr after + # we've raised (a caller retrying or reusing those buffers would + # otherwise race with them). + for f in futures: + f.cancel() + wait(futures) + raise # --------------------------------------------------------------------------- diff --git a/src/openptv2/thread_pool.py b/src/openptv2/thread_pool.py index e465c9bf..a7f0b51a 100644 --- a/src/openptv2/thread_pool.py +++ b/src/openptv2/thread_pool.py @@ -12,12 +12,23 @@ from concurrent.futures import ThreadPoolExecutor _executor: ThreadPoolExecutor | None = None +_creator_pid: int | None = None def get_executor() -> ThreadPoolExecutor: - """The process-wide shared executor, created on first use.""" - global _executor - if _executor is None: + """The process-wide shared executor, created on first use. + + Recreated whenever the current PID doesn't match the one that created + it: pyptv_batch_parallel forks worker processes on Unix, and a fork + only clones the calling thread, so a child inheriting an + already-initialized executor would have a pool object whose worker + threads never actually exist in that process -- submissions there + would have no consumer and hang forever. + """ + global _executor, _creator_pid + pid = os.getpid() + if _executor is None or _creator_pid != pid: _executor = ThreadPoolExecutor(max_workers=os.cpu_count() or 4) + _creator_pid = pid atexit.register(_executor.shutdown, wait=False, cancel_futures=True) return _executor