Stateful allocator support for concurrent_queue and concurrent_bounde… #1520
Conversation
There was a problem hiding this comment.
@YexuanXiao, many thanks for working on this gap in our containers!
| clear(); | ||
| if (queue_allocator_traits::propagate_on_container_move_assignment::value) { | ||
| my_allocator = std::move(other.my_allocator); | ||
| internal_swap(other); |
There was a problem hiding this comment.
I am afraid that with the full support for stateful allocators, propagation and uses-allocator construction, we cannot use internal_swap as an implementation for move semantics anymore:
Consider this->my_allocator to be stateful allocator.
After internal_swap, the memory allocated by *this would be transferred to other but the allocator with the correct state would not be transfered to *this (and it cannot be since the standard requires us to move-construct allocators.
Because of these, we will need a "fair" move semantics for both constructor and the assignment operator
There was a problem hiding this comment.
my_queue_representation is allocated by r1::cache_aligned_allocate instead of my_allocator and all memory allocated by my_allocator has been deallocated at line 153, so I don’t think this is an issue.
| using std::swap; | ||
| swap(my_allocator, other.my_allocator); | ||
| } else { | ||
| __TBB_ASSERT(my_allocator == other.my_allocator, "unequal allocators"); |
There was a problem hiding this comment.
Lets move it inside internal_swap
There was a problem hiding this comment.
I reviewed all the containers’ internal_swap, which typically have two overloads. I think they can be combined into a single function using if:
void internal_swap(concurrent_queue& src) {
if (!allocator_traits_type::queue_allocator_traits::propagate_on_container)
__TBB_ASSERT(my_allocator == src.my_allocator, "Swapping with unequal allocators is not allowed");
using std::swap;
swap(my_queue_representation, src.my_queue_representation);
}The compiler will optimize it and eliminate the unreachable branch.
Allocator helpers have the same pattern. Do we need to perform these cleanups, or should we keep things as they are?
…d_queue
Description
Add support for allocator propagation to concurrent_queue and concurrent_bounded_queue, and use the allocator’s construct function instead of placement new.
Type of change
Tests
Documentation
Breaks backward compatibility