Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new example that demonstrates using a thread-local allocator with beman::execution::task, and wires it into the examples build.
Changes:
- Add
examples/alloc-2.cppshowcasing a TLS-backedallocator_typefor tasks. - Register the new
alloc-2example target inexamples/CMakeLists.txt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| examples/alloc-2.cpp | New allocator example using thread_local state to select a std::pmr::memory_resource for task allocations. |
| examples/CMakeLists.txt | Adds alloc-2 to the list of built examples (non-MSVC). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,82 @@ | |||
| // examples/alloc-1.cpp -*-C++-*- | |||
There was a problem hiding this comment.
File header comment says examples/alloc-1.cpp but this is alloc-2.cpp, which can confuse readers and tooling that relies on the header banner. Update the comment to match the actual filename.
| // examples/alloc-1.cpp -*-C++-*- | |
| // examples/alloc-2.cpp -*-C++-*- |
| auto p = std::malloc(n); | ||
| std::cout << " global new(" << n << ")->" << p << "\n"; |
There was a problem hiding this comment.
The throwing global operator new(std::size_t) is required to throw std::bad_alloc on allocation failure; returning nullptr is undefined behavior for callers that assume a valid pointer. Consider checking the malloc result and throwing, or implement only the nothrow overload if you intend to return null.
| auto p = std::malloc(n); | |
| std::cout << " global new(" << n << ")->" << p << "\n"; | |
| void* p = std::malloc(n); | |
| std::cout << " global new(" << n << ")->" << p << "\n"; | |
| if (!p) { | |
| throw std::bad_alloc(); | |
| } |
| void* do_allocate(std::size_t n, std::size_t) override { | ||
| auto p{std::malloc(n)}; | ||
| std::cout << " resource::allocate(" << n << ")->" << p << "\n"; | ||
| return p; | ||
| } | ||
| void do_deallocate(void* p, std::size_t n, std::size_t) override { | ||
| std::cout << " resource::deallocate(" << p << ", " << n << ")\n"; | ||
| std::free(p); |
There was a problem hiding this comment.
std::pmr::memory_resource::do_allocate(bytes, alignment) must honor the requested alignment. Ignoring the alignment parameter and using malloc can return misaligned storage for over-aligned allocations. Use an aligned allocation strategy (e.g., ::operator new(bytes, std::align_val_t(alignment)) + matching aligned delete, or std::aligned_alloc where available).
| void* do_allocate(std::size_t n, std::size_t) override { | |
| auto p{std::malloc(n)}; | |
| std::cout << " resource::allocate(" << n << ")->" << p << "\n"; | |
| return p; | |
| } | |
| void do_deallocate(void* p, std::size_t n, std::size_t) override { | |
| std::cout << " resource::deallocate(" << p << ", " << n << ")\n"; | |
| std::free(p); | |
| void* do_allocate(std::size_t n, std::size_t alignment) override { | |
| void* p = nullptr; | |
| #if defined(__cpp_aligned_new) | |
| if (alignment > __STDCPP_DEFAULT_NEW_ALIGNMENT__) { | |
| p = ::operator new(n, std::align_val_t(alignment)); | |
| } else | |
| #endif | |
| { | |
| p = ::operator new(n); | |
| } | |
| std::cout << " resource::allocate(" << n << ")->" << p << "\n"; | |
| return p; | |
| } | |
| void do_deallocate(void* p, std::size_t n, std::size_t alignment) override { | |
| std::cout << " resource::deallocate(" << p << ", " << n << ")\n"; | |
| #if defined(__cpp_aligned_new) | |
| if (alignment > __STDCPP_DEFAULT_NEW_ALIGNMENT__) { | |
| ::operator delete(p, n, std::align_val_t(alignment)); | |
| } else | |
| #endif | |
| { | |
| ::operator delete(p, n); | |
| } |
| tls_allocator::set(&res); | ||
| ex::sync_wait([ac]() -> a_task<> { | ||
| auto result{co_await async_fun(ac)}; | ||
| std::cout << " result=" << result << "\n"; |
There was a problem hiding this comment.
tls_allocator::set(&res) stores a pointer to a stack resource in a thread_local that outlives res. Even though this example exits soon after, leaving a thread_local pointing at a soon-to-be-destroyed object is easy to copy/paste into real code and becomes a dangling pointer hazard. Consider resetting back to std::pmr::new_delete_resource() after the sync_wait, or wrap the change in an RAII guard object.
No description provided.