Two performance issues that work against their own stated intent.
1. The CuPy memory pool is dismantled on every single call.
hypyp/sync/kernels/_cuda_dispatch.py:84-85 and hypyp/sync/kernels/cuda_accorr.py:127-128:
# Explicit cleanup: force immediate GPU memory release (not relying on GC)
cp.get_default_memory_pool().free_all_blocks()
The comment presents this as hygiene, but free_all_blocks() returns every cached block to the driver, so the next call must go back through cudaMalloc, and cudaFree synchronises the device. The whole purpose of CuPy's pool is to avoid exactly that. In a 1000-iteration surrogate or permutation loop this is a recurring per-call cost plus a forced device sync.
This is the symmetric over-correction of the Metal leak fixed in #279: there, nothing was ever released; here, everything is released too eagerly. The pool already bounds memory — it does not need manual draining on the hot path.
Suggested fix: drop the per-call free_all_blocks(). If a long-running caller genuinely needs to reclaim device memory, expose that as an explicit opt-in rather than doing it implicitly on every metric evaluation.
2. Channel-pair index arrays are rebuilt from a Python double loop on every call.
Four places, all identical in shape:
hypyp/sync/kernels/_metal_dispatch.py:102-108
hypyp/sync/kernels/metal_accorr.py:129-135
hypyp/sync/kernels/_cuda_dispatch.py:61-67
hypyp/sync/kernels/cuda_accorr.py:104-110
idx_i, idx_j = [], []
for i in range(C):
for j in range(i, C):
idx_i.append(i)
idx_j.append(j)
For a 128-channel dyad (C = 256) that is 32,896 Python-level append calls per invocation, plus a fresh host→device transfer of the index buffers. It depends only on C, so it is trivially cacheable with @lru_cache on C (returning the arrays, and for CUDA the device arrays).
Also in the Metal path, s_flat.tobytes() / c_flat.tobytes() (_metal_dispatch.py:112-115) copies the entire signal on every call. Since the buffers use MTLResourceStorageModeShared, newBufferWithBytes_ could take the array buffer directly, or the buffers could be allocated once and reused when the shape is unchanged — relevant because surrogate loops call this repeatedly with identically-shaped data.
Two performance issues that work against their own stated intent.
1. The CuPy memory pool is dismantled on every single call.
hypyp/sync/kernels/_cuda_dispatch.py:84-85andhypyp/sync/kernels/cuda_accorr.py:127-128:The comment presents this as hygiene, but
free_all_blocks()returns every cached block to the driver, so the next call must go back throughcudaMalloc, andcudaFreesynchronises the device. The whole purpose of CuPy's pool is to avoid exactly that. In a 1000-iteration surrogate or permutation loop this is a recurring per-call cost plus a forced device sync.This is the symmetric over-correction of the Metal leak fixed in #279: there, nothing was ever released; here, everything is released too eagerly. The pool already bounds memory — it does not need manual draining on the hot path.
Suggested fix: drop the per-call
free_all_blocks(). If a long-running caller genuinely needs to reclaim device memory, expose that as an explicit opt-in rather than doing it implicitly on every metric evaluation.2. Channel-pair index arrays are rebuilt from a Python double loop on every call.
Four places, all identical in shape:
hypyp/sync/kernels/_metal_dispatch.py:102-108hypyp/sync/kernels/metal_accorr.py:129-135hypyp/sync/kernels/_cuda_dispatch.py:61-67hypyp/sync/kernels/cuda_accorr.py:104-110For a 128-channel dyad (C = 256) that is 32,896 Python-level
appendcalls per invocation, plus a fresh host→device transfer of the index buffers. It depends only onC, so it is trivially cacheable with@lru_cacheonC(returning the arrays, and for CUDA the device arrays).Also in the Metal path,
s_flat.tobytes()/c_flat.tobytes()(_metal_dispatch.py:112-115) copies the entire signal on every call. Since the buffers useMTLResourceStorageModeShared,newBufferWithBytes_could take the array buffer directly, or the buffers could be allocated once and reused when the shape is unchanged — relevant because surrogate loops call this repeatedly with identically-shaped data.