Skip to content
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.

Sign up for GitHub

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

Refactor allocator handling of contiguous_storage #3050

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Refactor allocator handling of contiguous_storage #3050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
273985f
Refactor allocator handling of contiguous_storage
bernhardmgruber Dec 4, 2024
84847d9
Update thrust/thrust/detail/contiguous_storage.h
bernhardmgruber Dec 5, 2024
c2c32a3
Drop noexcept
bernhardmgruber Dec 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 84 additions & 35 deletions thrust/thrust/detail/contiguous_storage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
#include <thrust/detail/execution_policy.h>
#include <thrust/iterator/detail/normal_iterator.h>

#include <cuda/std/utility>

THRUST_NAMESPACE_BEGIN

namespace detail
Expand All @@ -38,6 +40,13 @@ namespace detail
struct copy_allocator_t
{};

struct allocator_mismatch_on_swap : std::runtime_error
{
allocator_mismatch_on_swap()
: std::runtime_error("swap called on containers with allocators that propagate on swap, but compare non-equal")
{}
};

// XXX parameter T is redundant with parameter Alloc
template <typename T, typename Alloc>
class contiguous_storage
Expand Down Expand Up @@ -70,6 +79,8 @@ class contiguous_storage
_CCCL_EXEC_CHECK_DISABLE
_CCCL_HOST_DEVICE explicit contiguous_storage(copy_allocator_t, const contiguous_storage& other, size_type n);

contiguous_storage& operator=(const contiguous_storage& x) = delete;

_CCCL_EXEC_CHECK_DISABLE
_CCCL_HOST_DEVICE ~contiguous_storage();

Expand Down Expand Up @@ -100,7 +111,29 @@ class contiguous_storage

_CCCL_HOST_DEVICE void deallocate() noexcept;

_CCCL_HOST_DEVICE void swap(contiguous_storage& x);
_CCCL_EXEC_CHECK_DISABLE
_CCCL_HOST_DEVICE void swap(contiguous_storage& x)
{
using ::cuda::std::swap;
swap(m_begin, x.m_begin);
swap(m_size, x.m_size);

// From C++ standard [container.reqmts]
// If allocator_traits<allocator_type>::propagate_on_container_swap::value is true, then allocator_type
// shall meet the Cpp17Swappable requirements and the allocators of a and b shall also be exchanged by calling
// swap as described in [swappable.requirements]. Otherwise, the allocators shall not be swapped, and the behavior
// is undefined unless a.get_allocator() == b.get_allocator().
_CCCL_IF_CONSTEXPR (allocator_traits<Alloc>::propagate_on_container_swap::value)
{
swap(m_allocator, x.m_allocator);
}
else _CCCL_IF_CONSTEXPR (!allocator_traits<Alloc>::is_always_equal::value)
{
NV_IF_TARGET(NV_IS_DEVICE, (assert(m_allocator == other);), (if (m_allocator != x.m_allocator) {
throw allocator_mismatch_on_swap();
}));
}
}

_CCCL_HOST_DEVICE void value_initialize_n(iterator first, size_type n);

Expand All @@ -122,23 +155,42 @@ class contiguous_storage

_CCCL_HOST_DEVICE void destroy(iterator first, iterator last) noexcept;

_CCCL_HOST_DEVICE void deallocate_on_allocator_mismatch(const contiguous_storage& other) noexcept;
_CCCL_HOST_DEVICE void deallocate_on_allocator_mismatch(const contiguous_storage& other) noexcept
{
// TODO(bgruber): replace dispatch by if constexpr in C++17
integral_constant<bool, allocator_traits<Alloc>::propagate_on_container_copy_assignment::value> c;
deallocate_on_allocator_mismatch_dispatch(c, other);
}

_CCCL_HOST_DEVICE void
destroy_on_allocator_mismatch(const contiguous_storage& other, iterator first, iterator last) noexcept;
destroy_on_allocator_mismatch(const contiguous_storage& other, iterator first, iterator last) noexcept
{
// TODO(bgruber): replace dispatch by if constexpr in C++17
integral_constant<bool, allocator_traits<Alloc>::propagate_on_container_copy_assignment::value> c;
destroy_on_allocator_mismatch_dispatch(c, other, first, last);
}

_CCCL_HOST_DEVICE void set_allocator(const allocator_type& alloc);

_CCCL_HOST_DEVICE bool is_allocator_not_equal(const allocator_type& alloc) const;

_CCCL_HOST_DEVICE bool is_allocator_not_equal(const contiguous_storage& other) const;

_CCCL_HOST_DEVICE void propagate_allocator(const contiguous_storage& other);
_CCCL_EXEC_CHECK_DISABLE
_CCCL_HOST_DEVICE void propagate_allocator(const contiguous_storage& other)
{
_CCCL_IF_CONSTEXPR (allocator_traits<Alloc>::propagate_on_container_copy_assignment::value)
{
m_allocator = other.m_allocator;
}
}

_CCCL_HOST_DEVICE void propagate_allocator(contiguous_storage& other);
_CCCL_EXEC_CHECK_DISABLE
_CCCL_HOST_DEVICE void propagate_allocator(contiguous_storage& other)
{
_CCCL_IF_CONSTEXPR (allocator_traits<Alloc>::propagate_on_container_move_assignment::value)
{
m_allocator = ::cuda::std::move(other.m_allocator);
}
}

// allow move assignment for a sane implementation of allocator propagation
// on move assignment
_CCCL_HOST_DEVICE contiguous_storage& operator=(contiguous_storage&& other);

_CCCL_SYNTHESIZE_SEQUENCE_ACCESS(contiguous_storage, const_iterator);
Expand All @@ -151,34 +203,31 @@ class contiguous_storage

size_type m_size;

// disallow assignment
contiguous_storage& operator=(const contiguous_storage& x);

_CCCL_HOST_DEVICE void swap_allocators(true_type, const allocator_type&);

_CCCL_HOST_DEVICE void swap_allocators(false_type, allocator_type&);

_CCCL_HOST_DEVICE bool is_allocator_not_equal_dispatch(true_type, const allocator_type&) const;

_CCCL_HOST_DEVICE bool is_allocator_not_equal_dispatch(false_type, const allocator_type&) const;

_CCCL_HOST_DEVICE void deallocate_on_allocator_mismatch_dispatch(true_type, const contiguous_storage& other) noexcept;

_CCCL_HOST_DEVICE void deallocate_on_allocator_mismatch_dispatch(false_type, const contiguous_storage& other) noexcept;
_CCCL_EXEC_CHECK_DISABLE
_CCCL_HOST_DEVICE void deallocate_on_allocator_mismatch_dispatch(true_type, const contiguous_storage& other) noexcept
{
if (m_allocator != other.m_allocator)
{
deallocate();
}
}

_CCCL_HOST_DEVICE void destroy_on_allocator_mismatch_dispatch(
true_type, const contiguous_storage& other, iterator first, iterator last) noexcept;
_CCCL_HOST_DEVICE static void deallocate_on_allocator_mismatch_dispatch(false_type, const contiguous_storage&) noexcept
{}

_CCCL_EXEC_CHECK_DISABLE
_CCCL_HOST_DEVICE void destroy_on_allocator_mismatch_dispatch(
false_type, const contiguous_storage& other, iterator first, iterator last) noexcept;

_CCCL_HOST_DEVICE void propagate_allocator_dispatch(true_type, const contiguous_storage& other);

_CCCL_HOST_DEVICE void propagate_allocator_dispatch(false_type, const contiguous_storage& other);

_CCCL_HOST_DEVICE void propagate_allocator_dispatch(true_type, contiguous_storage& other);

_CCCL_HOST_DEVICE void propagate_allocator_dispatch(false_type, contiguous_storage& other);
true_type, const contiguous_storage& other, iterator first, iterator last) noexcept
{
if (m_allocator != other.m_allocator)
{
destroy(first, last);
}
} // end contiguous_storage::destroy_on_allocator_mismatch()

_CCCL_HOST_DEVICE static void destroy_on_allocator_mismatch_dispatch(
false_type, const contiguous_storage& other, iterator first, iterator last) noexcept
{}

friend _CCCL_HOST_DEVICE void swap(contiguous_storage& lhs, contiguous_storage& rhs) noexcept(noexcept(lhs.swap(rhs)))
{
Expand Down
172 changes: 8 additions & 164 deletions thrust/thrust/detail/contiguous_storage.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@
#elif defined(_CCCL_IMPLICIT_SYSTEM_HEADER_MSVC)
# pragma system_header
#endif // no system header
#include <thrust/detail/allocator/allocator_traits.h>

#include <thrust/detail/allocator/copy_construct_range.h>
#include <thrust/detail/allocator/destroy_range.h>
#include <thrust/detail/allocator/fill_construct_range.h>
#include <thrust/detail/allocator/value_initialize_range.h>
#include <thrust/detail/contiguous_storage.h>

#include <cuda/std/utility>

#include <stdexcept> // for std::runtime_error
#include <utility> // for use of std::swap in the WAR below

Expand All @@ -44,14 +42,6 @@ THRUST_NAMESPACE_BEGIN
namespace detail
{

class allocator_mismatch_on_swap : public std::runtime_error
{
public:
allocator_mismatch_on_swap()
: std::runtime_error("swap called on containers with allocators that propagate on swap, but compare non-equal")
{}
};

_CCCL_EXEC_CHECK_DISABLE
template <typename T, typename Alloc>
_CCCL_HOST_DEVICE contiguous_storage<T, Alloc>::contiguous_storage(const Alloc& alloc)
Expand Down Expand Up @@ -191,20 +181,6 @@ _CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::deallocate() noexcept
} // end if
} // end contiguous_storage::deallocate()

_CCCL_EXEC_CHECK_DISABLE
template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::swap(contiguous_storage& x)
{
using ::cuda::std::swap;
swap(m_begin, x.m_begin);
swap(m_size, x.m_size);

// FIXME(bgruber): swap_allocators already swaps m_allocator, so we are swapping twice here !!
swap_allocators(integral_constant<bool, allocator_traits<Alloc>::propagate_on_container_swap::value>(),
x.m_allocator);
swap(m_allocator, x.m_allocator);
} // end contiguous_storage::swap()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::value_initialize_n(iterator first, size_type n)
{
Expand Down Expand Up @@ -262,166 +238,34 @@ _CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::destroy(iterator first, ite
destroy_range(m_allocator, first.base(), last - first);
} // end contiguous_storage::destroy()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void
contiguous_storage<T, Alloc>::deallocate_on_allocator_mismatch(const contiguous_storage& other) noexcept
{
integral_constant<bool, allocator_traits<Alloc>::propagate_on_container_copy_assignment::value> c;

deallocate_on_allocator_mismatch_dispatch(c, other);
} // end contiguous_storage::deallocate_on_allocator_mismatch

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::destroy_on_allocator_mismatch(
const contiguous_storage& other, iterator first, iterator last) noexcept
{
integral_constant<bool, allocator_traits<Alloc>::propagate_on_container_copy_assignment::value> c;

destroy_on_allocator_mismatch_dispatch(c, other, first, last);
} // end contiguous_storage::destroy_on_allocator_mismatch

_CCCL_EXEC_CHECK_DISABLE
template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::set_allocator(const Alloc& alloc)
{
m_allocator = alloc;
} // end contiguous_storage::set_allocator()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE bool contiguous_storage<T, Alloc>::is_allocator_not_equal(const Alloc& alloc) const
{
return is_allocator_not_equal_dispatch(
integral_constant<bool, allocator_traits<Alloc>::is_always_equal::value>(), alloc);
} // end contiguous_storage::is_allocator_not_equal()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE bool
contiguous_storage<T, Alloc>::is_allocator_not_equal(const contiguous_storage<T, Alloc>& other) const
{
return is_allocator_not_equal(m_allocator, other.m_allocator);
} // end contiguous_storage::is_allocator_not_equal()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::propagate_allocator(const contiguous_storage& other)
{
integral_constant<bool, allocator_traits<Alloc>::propagate_on_container_copy_assignment::value> c;

propagate_allocator_dispatch(c, other);
} // end contiguous_storage::propagate_allocator()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::propagate_allocator(contiguous_storage& other)
{
integral_constant<bool, allocator_traits<Alloc>::propagate_on_container_move_assignment::value> c;

propagate_allocator_dispatch(c, other);
} // end contiguous_storage::propagate_allocator()

_CCCL_EXEC_CHECK_DISABLE
template <typename T, typename Alloc>
_CCCL_HOST_DEVICE contiguous_storage<T, Alloc>& contiguous_storage<T, Alloc>::operator=(contiguous_storage&& other)
{
if (size() > 0)
{
deallocate();
}
propagate_allocator(other);
_CCCL_IF_CONSTEXPR (allocator_traits<Alloc>::propagate_on_container_move_assignment::value)
{
m_allocator = ::cuda::std::move(other.m_allocator);
}

m_begin = std::move(other.m_begin);
m_size = std::move(other.m_size);

other.m_begin = pointer(static_cast<T*>(0));
other.m_size = 0;

return *this;
} // end contiguous_storage::propagate_allocator()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::swap_allocators(true_type, const Alloc&)
{} // end contiguous_storage::swap_allocators()

_CCCL_EXEC_CHECK_DISABLE
template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::swap_allocators(false_type, Alloc& other)
{
// FIXME(bgruber): it is really concerning, that swapping an allocator can throw. swap() should be noexcept in
// general.
NV_IF_TARGET(NV_IS_DEVICE,
(
// allocators must be equal when swapping containers with allocators that propagate on swap
assert(!is_allocator_not_equal(other));),
(if (is_allocator_not_equal(other)) { throw allocator_mismatch_on_swap(); }));
using ::cuda::std::swap;
swap(m_allocator, other);
} // end contiguous_storage::swap_allocators()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE bool
contiguous_storage<T, Alloc>::is_allocator_not_equal_dispatch(true_type /*is_always_equal*/, const Alloc&) const
{
return false;
} // end contiguous_storage::is_allocator_not_equal_dispatch()

_CCCL_EXEC_CHECK_DISABLE
template <typename T, typename Alloc>
_CCCL_HOST_DEVICE bool
contiguous_storage<T, Alloc>::is_allocator_not_equal_dispatch(false_type /*!is_always_equal*/, const Alloc& other) const
{
return m_allocator != other;
} // end contiguous_storage::is_allocator_not_equal_dispatch()

_CCCL_EXEC_CHECK_DISABLE
template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::deallocate_on_allocator_mismatch_dispatch(
true_type, const contiguous_storage& other) noexcept
{
if (m_allocator != other.m_allocator)
{
deallocate();
}
} // end contiguous_storage::deallocate_on_allocator_mismatch()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void
contiguous_storage<T, Alloc>::deallocate_on_allocator_mismatch_dispatch(false_type, const contiguous_storage&) noexcept
{} // end contiguous_storage::deallocate_on_allocator_mismatch()

_CCCL_EXEC_CHECK_DISABLE
template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::destroy_on_allocator_mismatch_dispatch(
true_type, const contiguous_storage& other, iterator first, iterator last) noexcept
{
if (m_allocator != other.m_allocator)
{
destroy(first, last);
}
} // end contiguous_storage::destroy_on_allocator_mismatch()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::destroy_on_allocator_mismatch_dispatch(
false_type, const contiguous_storage&, iterator, iterator) noexcept
{} // end contiguous_storage::destroy_on_allocator_mismatch()

_CCCL_EXEC_CHECK_DISABLE
template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void
contiguous_storage<T, Alloc>::propagate_allocator_dispatch(true_type, const contiguous_storage& other)
{
m_allocator = other.m_allocator;
} // end contiguous_storage::propagate_allocator()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::propagate_allocator_dispatch(false_type, const contiguous_storage&)
{} // end contiguous_storage::propagate_allocator()

_CCCL_EXEC_CHECK_DISABLE
template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::propagate_allocator_dispatch(true_type, contiguous_storage& other)
{
m_allocator = std::move(other.m_allocator);
} // end contiguous_storage::propagate_allocator()

template <typename T, typename Alloc>
_CCCL_HOST_DEVICE void contiguous_storage<T, Alloc>::propagate_allocator_dispatch(false_type, contiguous_storage&)
{} // end contiguous_storage::propagate_allocator()
}

} // namespace detail

Expand Down
Loading