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

Jump to bottom

Fixed memory_pool::min_block_size calculation #110

Closed
wants to merge 1 commit into from
Closed

Fixed memory_pool::min_block_size calculation #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 29 additions & 2 deletions include/foonathan/memory/detail/free_list.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "align.hpp"
#include "utility.hpp"
#include "debug_helpers.hpp"
#include "../config.hpp"

namespace foonathan
Expand All @@ -29,6 +30,11 @@ namespace foonathan
// alignment
static constexpr auto min_element_alignment = alignof(char*);

static constexpr std::size_t actual_node_size(std::size_t node_size) noexcept
{
return adjusted_node_size(node_size) + 2 * fence_size();
}

//=== constructor ===//
free_memory_list(std::size_t node_size) noexcept;

Expand Down Expand Up @@ -92,7 +98,16 @@ namespace foonathan
}

private:
std::size_t fence_size() const noexcept;

static constexpr std::size_t adjusted_node_size(std::size_t requested_size) noexcept
{
return requested_size > min_element_size ? requested_size : min_element_size;
}
static constexpr std::size_t fence_size() noexcept
{
return detail::debug_fence_size ? max_alignment : 0u;
}

void insert_impl(void* mem, std::size_t size) noexcept;

char* first_;
Expand All @@ -112,6 +127,11 @@ namespace foonathan
// alignment
static constexpr auto min_element_alignment = alignof(char*);

static constexpr std::size_t actual_node_size(std::size_t node_size) noexcept
{
return adjusted_node_size(node_size) + 2 * fence_size(adjusted_node_size(node_size));
}

//=== constructor ===//
ordered_free_memory_list(std::size_t node_size) noexcept;

Expand Down Expand Up @@ -186,7 +206,14 @@ namespace foonathan
}

private:
std::size_t fence_size() const noexcept;
static constexpr std::size_t adjusted_node_size(std::size_t requested_size) noexcept
{
return requested_size > min_element_size ? requested_size : min_element_size;
}
static constexpr std::size_t fence_size(std::size_t node_size) noexcept
{
return detail::debug_fence_size ? node_size : 0u;
}

// returns previous pointer
char* insert_impl(void* mem, std::size_t size) noexcept;
Expand Down
17 changes: 16 additions & 1 deletion include/foonathan/memory/detail/small_free_list.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "../config.hpp"
#include "utility.hpp"
#include "debug_helpers.hpp"

namespace foonathan
{
Expand Down Expand Up @@ -47,6 +48,11 @@ namespace foonathan
// alignment
static constexpr std::size_t min_element_alignment = 1;

static constexpr std::size_t actual_node_size(std::size_t node_size) noexcept
{
return adjusted_node_size(node_size) + 2 * fence_size(adjusted_node_size(node_size));
}

//=== constructor ===//
small_free_memory_list(std::size_t node_size) noexcept;

Expand Down Expand Up @@ -127,7 +133,16 @@ namespace foonathan
}

private:
std::size_t fence_size() const noexcept;

static constexpr std::size_t adjusted_node_size(std::size_t requested_size) noexcept
{
// mainly here for consistency with other free_list implementaions; min_element_size=1 makes it a no-op.
return requested_size;
}
static constexpr std::size_t fence_size(std::size_t node_size) noexcept
{
return detail::debug_fence_size ? node_size : 0u;
}

chunk* find_chunk_impl(std::size_t n = 1) noexcept;
chunk* find_chunk_impl(unsigned char* node, chunk_base* first,
Expand Down
7 changes: 3 additions & 4 deletions include/foonathan/memory/memory_pool.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ namespace foonathan
using allocator_type = make_block_allocator_t<BlockOrRawAllocator>;
using pool_type = PoolType;

// actual minimum node size, including any debug fence additions.
static constexpr std::size_t min_node_size =
FOONATHAN_IMPL_DEFINED(free_list::min_element_size);
FOONATHAN_IMPL_DEFINED(free_list::actual_node_size(free_list::min_element_size));
Comment on lines 59 to +60
Copy link
Contributor Author

@jwdevel jwdevel Apr 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This is one part I'm not 100% sure of. Is min_node_size meant to be the minimum amount that can be requested (in which case I should not change it), or is it meant to be the minimum you'll actually get, as it is in this update?


/// \returns The minimum block size required for certain number of \concept{concept_node,node}.
/// \requires \c node_size must be a valid \concept{concept_node,node size}
Expand All @@ -65,9 +66,7 @@ namespace foonathan
std::size_t number_of_nodes) noexcept
{
return detail::memory_block_stack::implementation_offset()
+ number_of_nodes
* (((node_size > min_node_size) ? node_size : min_node_size)
+ (detail::debug_fence_size ? 2 * detail::max_alignment : 0));
+ number_of_nodes * free_list::actual_node_size(node_size);
}

/// \effects Creates it by specifying the size each \concept{concept_node,node} will have,
Expand Down
40 changes: 14 additions & 26 deletions src/detail/free_list.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ constexpr std::size_t free_memory_list::min_element_alignment;

free_memory_list::free_memory_list(std::size_t node_size) noexcept
: first_(nullptr),
node_size_(node_size > min_element_size ? node_size : min_element_size),
node_size_(adjusted_node_size(node_size)),
capacity_(0u)
{
}
Expand Down Expand Up @@ -186,9 +186,9 @@ void* free_memory_list::allocate(std::size_t n) noexcept
if (n <= node_size_)
return allocate();

auto actual_size = node_size_ + 2 * fence_size();
auto actual_size = actual_node_size(node_size_);

auto i = list_search_array(first_, n + 2 * fence_size(), actual_size);
auto i = list_search_array(first_, actual_node_size(n), actual_size);
if (i.first == nullptr)
return nullptr;

Expand Down Expand Up @@ -217,7 +217,7 @@ void free_memory_list::deallocate(void* ptr, std::size_t n) noexcept
else
{
auto mem = debug_fill_free(ptr, n, fence_size());
insert_impl(mem, n + 2 * fence_size());
insert_impl(mem, actual_node_size(n));
}
}

Expand All @@ -226,15 +226,9 @@ std::size_t free_memory_list::alignment() const noexcept
return alignment_for(node_size_);
}

std::size_t free_memory_list::fence_size() const noexcept
{
// fence size is max alignment
return debug_fence_size ? max_alignment : 0u;
}

void free_memory_list::insert_impl(void* mem, std::size_t size) noexcept
{
auto actual_size = node_size_ + 2 * fence_size();
auto actual_size = actual_node_size(node_size_);
auto no_nodes = size / actual_size;
FOONATHAN_MEMORY_ASSERT(no_nodes > 0);

Expand Down Expand Up @@ -344,7 +338,7 @@ constexpr std::size_t ordered_free_memory_list::min_element_size;
constexpr std::size_t ordered_free_memory_list::min_element_alignment;

ordered_free_memory_list::ordered_free_memory_list(std::size_t node_size) noexcept
: node_size_(node_size > min_element_size ? node_size : min_element_size),
: node_size_(adjusted_node_size(node_size)),
capacity_(0u),
last_dealloc_(end_node()),
last_dealloc_prev_(begin_node())
Expand Down Expand Up @@ -457,7 +451,7 @@ void* ordered_free_memory_list::allocate() noexcept
FOONATHAN_MEMORY_ASSERT(last_dealloc_prev_ == prev);
}

return debug_fill_new(node, node_size_, fence_size());
return debug_fill_new(node, node_size_, fence_size(node_size_));
}

void* ordered_free_memory_list::allocate(std::size_t n) noexcept
Expand All @@ -467,9 +461,9 @@ void* ordered_free_memory_list::allocate(std::size_t n) noexcept
if (n <= node_size_)
return allocate();

auto actual_size = node_size_ + 2 * fence_size();
auto actual_size = actual_node_size(node_size_);

auto i = xor_list_search_array(begin_node(), end_node(), n + 2 * fence_size(), actual_size);
auto i = xor_list_search_array(begin_node(), end_node(), actual_node_size(n), actual_size);
if (i.first == nullptr)
return nullptr;

Expand All @@ -485,12 +479,12 @@ void* ordered_free_memory_list::allocate(std::size_t n) noexcept
last_dealloc_prev_ = i.prev;
}

return debug_fill_new(i.first, n, fence_size());
return debug_fill_new(i.first, n, fence_size(node_size_));
}

void ordered_free_memory_list::deallocate(void* ptr) noexcept
{
auto node = static_cast<char*>(debug_fill_free(ptr, node_size_, fence_size()));
auto node = static_cast<char*>(debug_fill_free(ptr, node_size_, fence_size(node_size_)));

auto p =
find_pos(allocator_info(FOONATHAN_MEMORY_LOG_PREFIX "::detail::ordered_free_memory_list",
Expand All @@ -510,8 +504,8 @@ void ordered_free_memory_list::deallocate(void* ptr, std::size_t n) noexcept
deallocate(ptr);
else
{
auto mem = debug_fill_free(ptr, n, fence_size());
auto prev = insert_impl(mem, n + 2 * fence_size());
auto mem = debug_fill_free(ptr, n, fence_size(node_size_));
auto prev = insert_impl(mem, actual_node_size(n));

last_dealloc_ = static_cast<char*>(mem);
last_dealloc_prev_ = prev;
Expand All @@ -523,15 +517,9 @@ std::size_t ordered_free_memory_list::alignment() const noexcept
return alignment_for(node_size_);
}

std::size_t ordered_free_memory_list::fence_size() const noexcept
{
// node size is fence size
return debug_fence_size ? node_size_ : 0u;
}

char* ordered_free_memory_list::insert_impl(void* mem, std::size_t size) noexcept
{
auto actual_size = node_size_ + 2 * fence_size();
auto actual_size = actual_node_size(node_size_);
auto no_nodes = size / actual_size;
FOONATHAN_MEMORY_ASSERT(no_nodes > 0);

Expand Down
22 changes: 8 additions & 14 deletions src/detail/small_free_list.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ void small_free_memory_list::insert(void* mem, std::size_t size) noexcept
FOONATHAN_MEMORY_ASSERT(is_aligned(mem, max_alignment));
debug_fill_internal(mem, size, false);

auto actual_size = node_size_ + 2 * fence_size();
auto actual_size = actual_node_size(node_size_);
auto total_chunk_size = chunk::memory_offset + actual_size * chunk::max_nodes;
auto align_buffer = align_offset(total_chunk_size, alignof(chunk));

Expand Down Expand Up @@ -294,7 +294,7 @@ void small_free_memory_list::insert(void* mem, std::size_t size) noexcept

std::size_t small_free_memory_list::usable_size(std::size_t size) const noexcept
{
auto actual_size = node_size_ + 2 * fence_size();
auto actual_size = actual_node_size(node_size_);
auto total_chunk_size = chunk::memory_offset + actual_size * chunk::max_nodes;
auto no_chunks = size / total_chunk_size;
auto remainder = size % total_chunk_size;
Expand All @@ -311,18 +311,18 @@ void* small_free_memory_list::allocate() noexcept

--capacity_;

auto mem = chunk->allocate(node_size_ + 2 * fence_size());
auto mem = chunk->allocate(actual_node_size(node_size_));
FOONATHAN_MEMORY_ASSERT(mem);
return detail::debug_fill_new(mem, node_size_, fence_size());
return detail::debug_fill_new(mem, node_size_, fence_size(node_size_));
}

void small_free_memory_list::deallocate(void* mem) noexcept
{
auto info =
allocator_info(FOONATHAN_MEMORY_LOG_PREFIX "::detail::small_free_memory_list", this);

auto actual_size = node_size_ + 2 * fence_size();
auto node = static_cast<unsigned char*>(detail::debug_fill_free(mem, node_size_, fence_size()));
auto actual_size = actual_node_size(node_size_);
auto node = static_cast<unsigned char*>(detail::debug_fill_free(mem, node_size_, fence_size(node_size_)));

auto chunk = find_chunk_impl(node);
dealloc_chunk_ = chunk;
Expand All @@ -347,12 +347,6 @@ std::size_t small_free_memory_list::alignment() const noexcept
return alignment_for(node_size_);
}

std::size_t small_free_memory_list::fence_size() const noexcept
{
// node size is fence size
return debug_fence_size ? node_size_ : 0u;
}

chunk* small_free_memory_list::find_chunk_impl(std::size_t n) noexcept
{
if (auto c = make_chunk(alloc_chunk_, n))
Expand Down Expand Up @@ -382,7 +376,7 @@ chunk* small_free_memory_list::find_chunk_impl(std::size_t n) noexcept
chunk* small_free_memory_list::find_chunk_impl(unsigned char* node, chunk_base* first,
chunk_base* last) noexcept
{
auto actual_size = node_size_ + 2 * fence_size();
auto actual_size = actual_node_size(node_size_);

do
{
Expand All @@ -399,7 +393,7 @@ chunk* small_free_memory_list::find_chunk_impl(unsigned char* node, chunk_base*

chunk* small_free_memory_list::find_chunk_impl(unsigned char* node) noexcept
{
auto actual_size = node_size_ + 2 * fence_size();
auto actual_size = actual_node_size(node_size_);

if (auto c = from_chunk(dealloc_chunk_, node, actual_size))
return c;
Expand Down
32 changes: 32 additions & 0 deletions test/memory_pool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// found in the top-level directory of this distribution.

#include "memory_pool.hpp"
#include "container.hpp"
#include "smart_ptr.hpp"

#include <algorithm>
#include <catch.hpp>
Expand Down Expand Up @@ -65,3 +67,33 @@ TEST_CASE("memory_pool", "[pool]")
}
REQUIRE(alloc.no_allocated() == 0u);
}

TEST_CASE("memory_pool min_block_size", "[pool]") {

using pool_type = memory_pool<>;

struct MyObj {
float value;
char data[100];
};
CAPTURE(alignof(MyObj));

CAPTURE(pool_type::pool_type::type::min_element_size);
CAPTURE(pool_type::pool_type::type::actual_node_size(pool_type::pool_type::type::min_element_size));
CAPTURE(pool_type::min_node_size);

auto nodesize = allocate_shared_node_size<MyObj, pool_type>::value;
CAPTURE(nodesize);

// Make a pool with the smallest block size possible.
auto minblock = pool_type::min_block_size(nodesize, 1);
CAPTURE(minblock);

// Create and use the pool
pool_type pool(nodesize, minblock);
auto ptr = allocate_shared<MyObj>(pool);

// These will grow the pool; should still succeed.
ptr = allocate_shared<MyObj>(pool);
ptr = allocate_shared<MyObj>(pool);
}