Skip to content

CUDA: free_all_blocks() on every call defeats the CuPy memory pool; pair indices rebuilt per call in all four dispatchers #309

Description

@Ramdam17

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions