-
Notifications
You must be signed in to change notification settings - Fork 25
Adds cleanup functions for resources #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
f954727
ac6630b
91f6622
db00687
300af67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~// | ||
| // Copyright (c) Lawrence Livermore National Security, LLC and other | ||
| // Camp Project Developers. See top-level LICENSE and COPYRIGHT | ||
| // files for dates and other details. No copyright assignment is required | ||
| // to contribute to Camp. | ||
| // | ||
| // SPDX-License-Identifier: (BSD-3-Clause) | ||
| //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~// | ||
|
|
||
| #ifndef CAMP_INIT_HELPERS_HPP | ||
| #define CAMP_INIT_HELPERS_HPP | ||
|
|
||
| #include <atomic> | ||
| #include <mutex> | ||
| #include <utility> | ||
|
|
||
| namespace camp | ||
| { | ||
|
|
||
| /// Resettable version of std::once_flag | ||
| /// | ||
| /// This is similar to std::once_flag used in std::call_once. However, | ||
| /// this version supports resetting flag, so that memory can be cleaned | ||
| /// and re-initialized at a later if needed. | ||
| /// | ||
| /// @note Currently, this uses atomic<bool> instead of atomic_flag to support | ||
| /// older versions of GCC. atomic_flag::test is not supported until | ||
| /// GCC 11. atomic_flag would be preferred as it is guaranteed to | ||
| /// not use a lock. | ||
| class resettable_once_flag | ||
| { | ||
| public: | ||
| resettable_once_flag() : m_lock{}, m_flag{false} | ||
| {} | ||
|
|
||
| resettable_once_flag(const resettable_once_flag&) = delete; | ||
| resettable_once_flag& operator=(const resettable_once_flag&) = delete; | ||
|
|
||
| resettable_once_flag(resettable_once_flag&&) = delete; | ||
| resettable_once_flag& operator=(resettable_once_flag&&) = delete; | ||
|
|
||
| bool test(std::memory_order order = std::memory_order_seq_cst) noexcept | ||
| { | ||
| return m_flag.load(order); | ||
| } | ||
|
|
||
| void set(bool value, std::memory_order order = std::memory_order_seq_cst) noexcept | ||
| { | ||
| m_flag.store(value, order); | ||
| } | ||
|
|
||
| void clear() | ||
| { | ||
| m_flag.store(false, std::memory_order_release); | ||
| } | ||
|
|
||
| template <typename Callable, typename... Args> | ||
| friend void call_once(camp::resettable_once_flag& flag, Callable&& callable, | ||
| Args&&... args); | ||
| private: | ||
| std::mutex m_lock; | ||
| std::atomic<bool> m_flag; | ||
| }; | ||
|
|
||
| /// Resettable version of std::call_once | ||
| /// | ||
| /// This is similar to std::call_once. However, this version supports | ||
| /// uses a custom ``camp::resettable_once_flag`` that allows the | ||
| /// data to be cleaned up. If needed, calling this function again after | ||
| /// resetting the flag will call the callable function again. | ||
| template <typename Callable, typename... Args> | ||
| void call_once(camp::resettable_once_flag& flag, Callable&& callable, Args&&... args) | ||
| { | ||
| if (flag.test(std::memory_order::acquire)) [[likely]] { | ||
| return; | ||
| } | ||
|
|
||
| std::lock_guard guard(flag.m_lock); | ||
| if (!flag.test(std::memory_order::relaxed)) { | ||
| callable(std::forward<Args>(args)...); | ||
| flag.set(true, std::memory_order::release); | ||
| } | ||
| } | ||
|
|
||
| } // namespace camp | ||
|
|
||
| #endif // CAMP_INIT_HELPERS_HPP |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
|
|
||
| #include "camp/defines.hpp" | ||
| #include "camp/helpers.hpp" | ||
| #include "camp/init_helpers.hpp" | ||
| #include "camp/resource/event.hpp" | ||
| #include "camp/resource/platform.hpp" | ||
|
|
||
|
|
@@ -152,27 +153,39 @@ namespace resources | |
|
|
||
| class Cuda | ||
| { | ||
| static constexpr int num_streams = 16; | ||
|
|
||
| struct stream_state { | ||
| std::array<cudaStream_t, num_streams> streams{}; | ||
| cudaStream_t default_stream = nullptr; | ||
| int previous = num_streams - 1; | ||
| camp::resettable_once_flag flag; | ||
| }; | ||
|
|
||
| static stream_state& get_stream_state() | ||
| { | ||
| static stream_state state; | ||
| return state; | ||
| } | ||
|
|
||
| static cudaStream_t get_a_stream(int num) | ||
| { | ||
| static constexpr int num_streams = 16; | ||
| static std::array<cudaStream_t, num_streams> s_streams = [] { | ||
| std::array<cudaStream_t, num_streams> streams; | ||
| for (auto& s : streams) { | ||
| CAMP_CUDA_API_INVOKE_AND_CHECK(cudaStreamCreate, &s); | ||
| } | ||
| return streams; | ||
| }(); | ||
| auto& state = get_stream_state(); | ||
|
|
||
| static std::mutex s_mtx; | ||
| static int s_previous = num_streams - 1; | ||
| camp::call_once(state.flag, [&] () { | ||
| for (auto& s : state.streams) { | ||
| if (s == nullptr) { | ||
| CAMP_CUDA_API_INVOKE_AND_CHECK(cudaStreamCreate, &s); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| if (num < 0) { | ||
| std::lock_guard<std::mutex> lock(s_mtx); | ||
| s_previous = (s_previous + 1) % num_streams; | ||
| return s_streams[s_previous]; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still needs to be locked.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact does it make sense to make this whole function a member function of the state now?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It probably makes sense to make getting a stream and getting the default stream member functions at this point. |
||
| state.previous = (state.previous + 1) % num_streams; | ||
| return state.streams[state.previous]; | ||
| } | ||
|
|
||
| return s_streams[num % num_streams]; | ||
| return state.streams[num % num_streams]; | ||
| } | ||
|
|
||
| // Private from-stream constructor | ||
|
|
@@ -226,16 +239,54 @@ namespace resources | |
|
|
||
| static Cuda get_default() | ||
| { | ||
| static Cuda c([] { | ||
| cudaStream_t s; | ||
| #if CAMP_USE_PLATFORM_DEFAULT_STREAM | ||
| s = 0; | ||
| return Cuda(nullptr); | ||
| #else | ||
| CAMP_CUDA_API_INVOKE_AND_CHECK(cudaStreamCreate, &s); | ||
| auto& state = get_stream_state(); | ||
|
|
||
| camp::call_once(state.flag, [&] () { | ||
| if (state.default_stream == nullptr) { | ||
| CAMP_CUDA_API_INVOKE_AND_CHECK(cudaStreamCreate, | ||
| &state.default_stream); | ||
| } | ||
| }); | ||
| return Cuda(state.default_stream); | ||
|
MrBurmark marked this conversation as resolved.
|
||
| #endif | ||
| return s; | ||
| }()); | ||
| return c; | ||
| } | ||
|
|
||
| /** | ||
| * \brief Destroy all CUDA streams created and managed by CAMP. | ||
| * | ||
| * Existing resources that refer to CAMP-managed streams are invalid | ||
| * after this call. Streams passed to CudaFromStream and the CUDA | ||
| * platform default stream are not destroyed. This function may be | ||
| * called repeatedly, and later resource construction recreates the | ||
| * managed streams. | ||
| * | ||
| * The caller must ensure no other thread is using CUDA resources while | ||
| * cleanup runs. | ||
| */ | ||
| static void cleanup() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it make more sense to put most of this code in a member function of the state?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can move most of the cleanup logic to be a state member function. However, I think we will still want a static cleanup function on the resource to be consistent across resources. |
||
| { | ||
| auto& state = get_stream_state(); | ||
|
|
||
| for (auto& s : state.streams) { | ||
| if (s != nullptr) { | ||
| CAMP_CUDA_API_INVOKE_AND_CHECK(cudaStreamDestroy, s); | ||
| s = nullptr; | ||
| } | ||
| } | ||
| state.previous = num_streams - 1; | ||
|
|
||
| #if !CAMP_USE_PLATFORM_DEFAULT_STREAM | ||
| if (state.default_stream != nullptr) { | ||
| CAMP_CUDA_API_INVOKE_AND_CHECK(cudaStreamDestroy, | ||
| state.default_stream); | ||
| state.default_stream = nullptr; | ||
| } | ||
| #endif | ||
|
|
||
| state.flag.clear(); | ||
| } | ||
|
|
||
| CudaEvent get_event() | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that we're manually handling the thread safety now, we should probably make this a class static member variable so it doesn't get another layer of thread safe initialization that comes from being a function local static. Given that we have a library we should probably put the variable definition in the camp sources, as opposed to an inline static member var.