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.

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

[FIX] Fixes aligned allocator issue for non posix implementation of aligned_alloc. #1362

Merged
merged 4 commits into from
Dec 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
138 changes: 92 additions & 46 deletions include/seqan3/range/container/aligned_allocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,22 @@ namespace seqan3
{

/*!\brief Allocates uninitialized storage whose memory-alignment is specified by *alignment*.
* \tparam value_t \copydoc aligned_allocator::value_type
* \tparam alignment_v \copydoc aligned_allocator::alignment
* \tparam value_t The value type of the allocation.
* \tparam alignment_v The memory-alignment of the allocation; defaults to `__STDCPP_DEFAULT_NEW_ALIGNMENT__`.
* \ingroup container
*
* \details
*
* This class allocates memory at the given `alignment_v` offset. This makes sure that the allocated memory
* starts at a memory offset equal to some multiple of the word size. More formally, a memory address `a`, is said to
* be `n`-byte aligned when `n` is a power of two and `a` is a multiple of `n` bytes.
*
* If the specified `alignment` is not supported (e.g. alignments that are not a power of two) by the
* used allocation method a std::bad_alloc exception will be thrown. For requested alignments larger than
* `__STDCPP_DEFAULT_NEW_ALIGNMENT__`, also called new-extended alignments, the storage will have the alignment
* specified by the value `alignment`. Otherwise, the storage is aligned for any object that does not have new-extended
* alignment, e.g. `int` or `double`, and is of the requested size.
*
* \include test/snippet/range/container/aligned_allocator.cpp
*
* Will output something like:
marehr marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -59,16 +69,16 @@ template <typename value_t, size_t alignment_v = __STDCPP_DEFAULT_NEW_ALIGNMENT_
class aligned_allocator
{
public:
//!\brief The memory-alignment of the allocation
//!\brief The memory-alignment of the allocation.
static constexpr size_t alignment = alignment_v;
smehringer marked this conversation as resolved.
Show resolved Hide resolved

//!\brief The value type of the allocation
//!\brief The value type of the allocation.
using value_type = value_t;
//!\brief The pointer type of the allocation
//!\brief The pointer type of the allocation.
using pointer = value_type*;
//!\brief The difference type of the allocation
//!\brief The difference type of the allocation.
using difference_type = typename std::pointer_traits<pointer>::difference_type;
//!\brief The size type of the allocation
//!\brief The size type of the allocation.
using size_type = std::make_unsigned_t<difference_type>;

//!\brief Are any two allocators of the same aligned_allocator type always compare equal?
Expand All @@ -84,72 +94,108 @@ class aligned_allocator
aligned_allocator& operator=(aligned_allocator &&) = default; //!< Defaulted.
~aligned_allocator() = default; //!< Defaulted.

//!\brief Copy constructor with different value type.
template <class other_value_type>
constexpr aligned_allocator(aligned_allocator<other_value_type, alignment> const &) noexcept
//!\brief Copy constructor with different value type and alignment.
template <class other_value_type, size_t other_alignment>
constexpr aligned_allocator(aligned_allocator<other_value_type, other_alignment> const &) noexcept
{}
//!\}

/*!\brief Allocates `n * sizeof(T)` bytes of uninitialized storage by calling std::aligned_alloc, but it is
* unspecified when and how this function is called.
* \throws Throws std::bad_alloc if allocation fails.
/*!\brief Allocates sufficiently large memory to hold `n` many elements of `value_type`.
*
* \param[in] n The number of elements for which to allocate the memory.
*
* \returns The pointer to the first block of allocated memory.
*
* \throws Throws std::bad_alloc if allocation fails, i.e. either the call to the throwing version of
* operator new function throws or the requested memory exceeds the maximal number of elements to allocate.
*
* \details
*
* Allocates `n * sizeof(value_type)` bytes of uninitialized storage by calling
* [operator new](https://en.cppreference.com/w/cpp/memory/new/operator_new).
* If the given `alignment` is bigger than
* [__STDCPP_DEFAULT_NEW_ALIGNMENT__](https://en.cppreference.com/w/cpp/memory/new/align_val_t), the alignment
* aware operator new that takes as second argument the desired alignment of type std::align_val_t is used.
*
* \note We call the new operator with the semantic requirements that the c++ standard specifies/demands, but be
* aware that users can overload any (global) `operator new` that might not adhere to the standard and might
* cause std::bad_alloc or unaligned pointers.
*
* \sa https://en.cppreference.com/w/cpp/memory/allocator/allocate
*
* ### Thread safety
*
* Thread-safe.
*
* ### Exception
*
* Strong exception guarantee.
*/
[[nodiscard]]
pointer allocate(size_type n)
pointer allocate(size_type const n) const
{
constexpr size_type max_size = std::numeric_limits<size_type>::max() / sizeof(value_type);
if (n > max_size)
throw std::bad_alloc();

// NOTE: On macOS glibc does not implement aligned_alloc, so we need to fallback to posix_memalign instead.
#if defined(__APPLE__) && (!defined(_GLIBCXX_HAVE_ALIGNED_ALLOC) && !defined(_ISOC11_SOURCE))
void * p{};
if (int res = posix_memalign(&p, alignment, n * sizeof(value_type)); res == 0 && p != nullptr)
return static_cast<pointer>(p);
#else
// NOTE:
// Allocate size bytes of uninitialized storage whose alignment is
// specified by alignment. The size parameter must be an integral
// multiple of alignment.
//
// Passing a size which is not an integral multiple of alignment or an
// alignment which is not valid or not supported by the implementation
// causes the function to fail and return a null pointer (C11, as
// published, specified undefined behavior in this case, this was
// corrected by DR 460).
// https://en.cppreference.com/w/cpp/memory/c/aligned_alloc
if (auto p = static_cast<pointer>(std::aligned_alloc(alignment, n * sizeof(value_type))))
return p;
#endif

throw std::bad_alloc();
throw std::bad_alloc{};
marehr marked this conversation as resolved.
Show resolved Hide resolved

size_t bytes_to_allocate = n * sizeof(value_type);
if constexpr (alignment <= __STDCPP_DEFAULT_NEW_ALIGNMENT__)
return static_cast<pointer>(::operator new(bytes_to_allocate));
else // Use alignment aware allocator function.
return static_cast<pointer>(::operator new(bytes_to_allocate, static_cast<std::align_val_t>(alignment)));
marehr marked this conversation as resolved.
Show resolved Hide resolved
}

/*!\brief Deallocates the storage referenced by the pointer p, which must be a pointer obtained by an earlier call
* to allocate().
* to seqan3::aligned_allocator::allocate.
*
* \param[in] p The pointer to the memory to be deallocated.
* \param[in] n The number of elements to be deallocated.
*
* \details
*
* The argument n must be equal to the first argument of the call to allocate() that originally produced p;
* otherwise, the behavior is undefined.
* The argument `n` must be equal to the first argument of the call to seqan3::aligned_allocator::allocate that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems error prone? Can't we cache n and remove the parameter from this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is defined and documented according to the allocator interface requirements: https://en.cppreference.com/w/cpp/named_req/Allocator, and I am afraid we need to keep this interface.

* originally produced `p`, otherwise the behavior is undefined. This function calls
* [operator delete](https://en.cppreference.com/w/cpp/memory/new/operator_delete) to deallocate the memory of
* specified size.
* If the given `alignment` is bigger than
* [__STDCPP_DEFAULT_NEW_ALIGNMENT__](https://en.cppreference.com/w/cpp/memory/new/align_val_t) the alignment
* aware operator delete that takes as third argument the alignment as std::align_val_t.
*
* Calls std::free, but it is unspecified when and how it is called.
* \sa https://en.cppreference.com/w/cpp/memory/allocator/deallocate
*
* ### Thread safety
*
* Thread-safe.
*
* ### Exception
*
* Nothrow guarantee.
*/
void deallocate(pointer p, size_type) noexcept
void deallocate(pointer const p, size_type const n) const noexcept
{
std::free(p);
if constexpr (alignment <= __STDCPP_DEFAULT_NEW_ALIGNMENT__)
::operator delete(p, n);
else // Use alignment aware deallocator function.
::operator delete(p, n, static_cast<std::align_val_t>(alignment));
}

/*!\brief The aligned_allocator member template class aligned_allocator::rebind provides a way to obtain an
* allocator for a different type.
* allocator for a different type.
*
* \tparam new_value_type The other value type.
*
* \details
*
* If the alignment of the new type exceeds the alignment of the current allocator, the larger alignment will
* be used.
*/
template <typename new_value_type>
struct rebind
{
//!\brief The alignment for the rebound allocator.
static constexpr size_t other_alignment = std::max(alignof(new_value_type), alignment);
//!\brief The type of the allocator for a different value type.
using other = aligned_allocator<new_value_type, alignment>;
using other = aligned_allocator<new_value_type, other_alignment>;
};

/*!\name Comparison operators
Expand Down
73 changes: 71 additions & 2 deletions test/unit/range/container/aligned_allocator_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@

#include <gtest/gtest.h>

#include <seqan3/range/container/aligned_allocator.hpp>

#include <deque>
#include <list>
#include <map>
#include <vector>

#include <seqan3/core/bit_manipulation.hpp>
#include <seqan3/range/container/aligned_allocator.hpp>

using namespace seqan3;

// standard construction.
Expand Down Expand Up @@ -47,6 +48,12 @@ TEST(aligned_allocator, conversion_constructor)
[[maybe_unused]] aligned_allocator<float, 16> float_alloc{int_alloc};
}

TEST(aligned_allocator, request_too_much_memory)
{
aligned_allocator<int, 16> alloc{};
EXPECT_THROW((void) alloc.allocate(std::numeric_limits<uint64_t>::max()), std::bad_alloc);
}

size_t memory_alignment(void * value, size_t alignment)
{
return (((size_t)value) & (alignment-1));
Expand Down Expand Up @@ -80,6 +87,68 @@ TEST(aligned_allocator, memory_alignment)
alloc.deallocate(begin, size);
}

TEST(aligned_allocator, memory_alignment_bigger_than_default_new_alignment)
{
size_t size = 10;
constexpr size_t alignment = detail::next_power_of_two(__STDCPP_DEFAULT_NEW_ALIGNMENT__ + 1);
aligned_allocator<int, alignment> alloc{};

int * begin = alloc.allocate(size);
int * end = begin + size;

EXPECT_EQ(sizeof(int), 4u);

EXPECT_EQ(memory_alignment(begin, alignment), 0u);
EXPECT_EQ(memory_alignment(end, alignment), 8u);

EXPECT_EQ(memory_alignment(begin + 1, alignment), 4u);
EXPECT_EQ(memory_alignment(begin + 2, alignment), 8u);
EXPECT_EQ(memory_alignment(begin + 3, alignment), 12u);
EXPECT_EQ(memory_alignment(begin + 4, alignment), 16u);
EXPECT_EQ(memory_alignment(begin + 5, alignment), 20u);
EXPECT_EQ(memory_alignment(begin + 6, alignment), 24u);
EXPECT_EQ(memory_alignment(begin + 7, alignment), 28u);
EXPECT_EQ(memory_alignment(begin + 8, alignment), 0u);
EXPECT_EQ(memory_alignment(begin + 9, alignment), 4u);
EXPECT_EQ(memory_alignment(begin + 10, alignment), 8u);

alloc.deallocate(begin, size);
}

struct large_alignment
{
alignas(64) std::array<int32_t, 2> data{};
};

TEST(aligned_allocator, memory_alignment_with_large_alignment_type)
{
size_t size = 10;
aligned_allocator<large_alignment, alignof(large_alignment)> alloc{};

large_alignment * begin = alloc.allocate(size);
large_alignment * end = begin + size;

constexpr size_t alignment = alignof(large_alignment);
EXPECT_EQ(sizeof(large_alignment), 64u);
EXPECT_EQ(alignof(large_alignment), 64u);

EXPECT_EQ(memory_alignment(begin, alignment), 0u);
EXPECT_EQ(memory_alignment(end, alignment), 0u);

EXPECT_EQ(memory_alignment(begin + 1, alignment), 0u);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it always 0? I don't want to think :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it does not use a larger alignment than the alignment of the type that is allocated for (which is 64 bytes). So every value starts at 64 byte boundary. Also that's why I called this test default to show the behavior of a new extended type where we use the default alignment of the type and not a larger one.

EXPECT_EQ(memory_alignment(begin + 2, alignment), 0u);
EXPECT_EQ(memory_alignment(begin + 3, alignment), 0u);
EXPECT_EQ(memory_alignment(begin + 4, alignment), 0u);
EXPECT_EQ(memory_alignment(begin + 5, alignment), 0u);
EXPECT_EQ(memory_alignment(begin + 6, alignment), 0u);
EXPECT_EQ(memory_alignment(begin + 7, alignment), 0u);
EXPECT_EQ(memory_alignment(begin + 8, alignment), 0u);
EXPECT_EQ(memory_alignment(begin + 9, alignment), 0u);
EXPECT_EQ(memory_alignment(begin + 10, alignment), 0u);

alloc.deallocate(begin, size);
}

TEST(aligned_allocator, in_vector)
{
size_t size = 10;
Expand Down