Synchronize CUDA async deallocate_sync

[bdice]

Description

Ensures CUDA async memory resources wait for async frees before returning from deallocate_sync(). The sync wrappers now deallocate on the default stream and synchronize that same stream before returning.

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7b98c2c4-979d-4063-abe1-07530a8fd59c

📥 Commits

Reviewing files that changed from the base of the PR and between ec97f1a and 9337403.

📒 Files selected for processing (3)

• cpp/include/rmm/mr/cuda_async_view_memory_resource.hpp
• cpp/src/mr/detail/cuda_async_managed_memory_resource_impl.cpp
• cpp/src/mr/detail/cuda_async_memory_resource_impl.cpp

---

📝 Walkthrough

Summary by CodeRabbit

• Refactor
  • Updated stream synchronization handling in CUDA memory resource deallocation across multiple internal resource implementations.

Walkthrough

Three deallocate_sync methods across async memory resource implementations now construct a local cuda::stream_ref for cudaStream_t{nullptr}, reuse it for the internal deallocate call, and synchronize using cudaStreamSynchronize(stream.get()).

Changes

Stream Reference Consistency Refactoring

Layer / File(s)	Summary
Stream reference reuse in deallocate_sync cpp/include/rmm/mr/cuda_async_view_memory_resource.hpp, cpp/src/mr/detail/cuda_async_managed_memory_resource_impl.cpp, cpp/src/mr/detail/cuda_async_memory_resource_impl.cpp	Three deallocate_sync implementations updated to create and reuse a single cuda::stream_ref for cudaStream_t{nullptr} when calling deallocate and to call cudaStreamSynchronize(stream.get()) for synchronization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

• vyasr
• nirandaperera
• wence-

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: improving synchronization in the CUDA async deallocate_sync function across multiple memory resource implementations.
Description check	✅ Passed	The description is directly related to the changeset, explaining the purpose of ensuring CUDA async memory resources wait for async frees before returning from deallocate_sync().
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[davidwendt]

LGTM.
Seems all of these function definitions could be moved to a .cpp file.

[wence-]

question: what is the point of this synchronisation? It can't really be stream safety because after this call the ptr is not alive and cannot be reused. Is it that you want the freed memory to be immediately available on all streams?

[bdice]

Yes, the name says "sync" so we should guarantee there is no asynchronous work ordered after the end of this function's scope. This should align with the allocation behavior.

[bdice]

Seems all of these function definitions could be moved to a .cpp file.

I agree, this resource doesn't match the others for some reason. I'll file a follow-up to fix that.

edit: Tracking in #2414

[bdice]

/merge