Skip to content

Commit

Permalink
Remove at_least
Browse files Browse the repository at this point in the history
The Pal was providing policy for overallocating a block of memory to
achieve alignment make that part of the backend.
The backend should be responsible for layout policy.
  • Loading branch information
mjp41 committed Jul 21, 2021
1 parent 9df0101 commit 5d0ae71
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 68 deletions.
6 changes: 3 additions & 3 deletions docs/PORTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ pages, rather than zeroing them synchronously in this call
```c++
template<bool committed>
static void* reserve_aligned(size_t size) noexcept;
static std::pair<void*, size_t> reserve_at_least(size_t size) noexcept;
static void* reserve(size_t size) noexcept;
```
All platforms should provide `reserve_at_least` and can optionally provide
All platforms should provide `reserve` and can optionally provide
`reserve_aligned` if the underlying system can provide strongly aligned
memory regions.
If the system guarantees only page alignment, implement only the second. The Pal is
free to overallocate based on the platform's desire and snmalloc
will find suitably aligned blocks inside the region. `reserve_at_least` should
will find suitably aligned blocks inside the region. `reserve` should
not commit memory as snmalloc will commit the range of memory it requires of what
is returned.
Expand Down
21 changes: 15 additions & 6 deletions src/backend/address_space.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,21 @@ namespace snmalloc
else if constexpr (!pal_supports<NoAllocation, PAL>)
{
// Need at least 2 times the space to guarantee alignment.
// Hold lock here as a race could cause additional requests to
// the PAL, and this could lead to suprious OOM. This is
// particularly bad if the PAL gives all the memory on first call.
auto block_and_size = PAL::reserve_at_least(size * 2);
block = CapPtr<void, CBChunk>(block_and_size.first);
block_size = block_and_size.second;
size_t needed_size = size * 2;
// Magic number (27) for over-allocating a block of memory
// These should be further refined based on experiments.
constexpr size_t min_size = bits::one_at_bit(27);
for (size_t size_request = bits::max(needed_size, min_size);
size_request >= needed_size;
size_request = size_request / 2)
{
block = CapPtr<void, CBChunk>(PAL::reserve(size_request));
if (block != nullptr)
{
block_size = size_request;
break;
}
}

// Ensure block is pointer aligned.
if (
Expand Down
2 changes: 1 addition & 1 deletion src/backend/backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace snmalloc
* This class implements the standard backend for handling allocations.
* It abstracts page table management and address space management.
*/
template<typename PAL, bool fixed_range>
template<SNMALLOC_CONCEPT(ConceptPAL) PAL, bool fixed_range>
class BackendAllocator
{
public:
Expand Down
4 changes: 1 addition & 3 deletions src/backend/pagemap.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,7 @@ namespace snmalloc

// TODO request additional space, and move to random offset.

// TODO wasting space if size2 bigger than needed.
auto [new_body_untyped, size2] =
Pal::reserve_at_least(ENTRIES * sizeof(T));
auto new_body_untyped = Pal::reserve(ENTRIES * sizeof(T));

auto new_body = reinterpret_cast<T*>(new_body_untyped);

Expand Down
16 changes: 8 additions & 8 deletions src/pal/pal_concept.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,10 @@ namespace snmalloc
* Absent any feature flags, the PAL must support a crude primitive allocator
*/
template<typename PAL>
concept ConceptPAL_reserve_at_least =
requires(PAL p, void* vp, std::size_t sz)
concept ConceptPAL_reserve =
requires(PAL p, std::size_t sz)
{
{ PAL::reserve_at_least(sz) } noexcept
-> ConceptSame<std::pair<void*, std::size_t>>;
{ PAL::reserve(sz) } noexcept -> ConceptSame<void*>;
};

/**
Expand Down Expand Up @@ -102,10 +101,11 @@ namespace snmalloc
(!pal_supports<LowMemoryNotification, PAL> ||
ConceptPAL_mem_low_notify<PAL>) &&
(pal_supports<NoAllocation, PAL> ||
(pal_supports<AlignedAllocation, PAL> &&
ConceptPAL_reserve_aligned<PAL>) ||
(!pal_supports<AlignedAllocation, PAL> &&
ConceptPAL_reserve_at_least<PAL>));
(
(!pal_supports<AlignedAllocation, PAL> ||
ConceptPAL_reserve_aligned<PAL>) &&
ConceptPAL_reserve<PAL>)
);

} // namespace snmalloc
#endif
40 changes: 14 additions & 26 deletions src/pal/pal_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,44 +253,32 @@ namespace snmalloc
* POSIX does not define a portable interface for specifying alignment
* greater than a page.
*/
static std::pair<void*, size_t> reserve_at_least(size_t size) noexcept
static void* reserve(size_t size) noexcept
{
SNMALLOC_ASSERT(bits::is_pow2(size));

// Magic number for over-allocating chosen by the Pal
// These should be further refined based on experiments.
constexpr size_t min_size =
bits::is64() ? bits::one_at_bit(31) : bits::one_at_bit(27);

#ifdef SNMALLOC_CHECK_CLIENT
auto prot = PROT_NONE;
#else
auto prot = PROT_READ | PROT_WRITE;
#endif

for (size_t size_request = bits::max(size, min_size);
size_request >= size;
size_request = size_request / 2)
{
void* p = mmap(
nullptr,
size_request,
prot,
MAP_PRIVATE | MAP_ANONYMOUS | DefaultMMAPFlags<OS>::flags,
AnonFD<OS>::fd,
0);
void* p = mmap(
nullptr,
size,
prot,
MAP_PRIVATE | MAP_ANONYMOUS | DefaultMMAPFlags<OS>::flags,
AnonFD<OS>::fd,
0);

if (p != MAP_FAILED)
{
if (p != MAP_FAILED)
{
#ifdef SNMALLOC_TRACING
std::cout << "Pal_posix reserved: " << p << " (" << size_request
<< ")" << std::endl;
std::cout << "Pal_posix reserved: " << p << " (" << size << ")"
<< std::endl;
#endif
return {p, size_request};
}
return p;
}

OS::error("Out of memory");
return nullptr;
}

/**
Expand Down
21 changes: 2 additions & 19 deletions src/pal/pal_windows.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,26 +185,9 @@ namespace snmalloc
}
# endif

static std::pair<void*, size_t> reserve_at_least(size_t size) noexcept
static void* reserve(size_t size) noexcept
{
SNMALLOC_ASSERT(bits::is_pow2(size));

// Magic number for over-allocating chosen by the Pal
// These should be further refined based on experiments.
constexpr size_t min_size =
bits::is64() ? bits::one_at_bit(32) : bits::one_at_bit(28);
for (size_t size_request = bits::max(size, min_size);
size_request >= size;
size_request = size_request / 2)
{
void* ret =
VirtualAlloc(nullptr, size_request, MEM_RESERVE, PAGE_READWRITE);
if (ret != nullptr)
{
return std::pair(ret, size_request);
}
}
error("Failed to allocate memory\n");
return VirtualAlloc(nullptr, size, MEM_RESERVE, PAGE_READWRITE);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/test/func/fixed_region/fixed_region.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ int main()
// 28 is large enough to produce a nested allocator.
// It is also large enough for the example to run in.
// For 1MiB superslabs, SUPERSLAB_BITS + 4 is not big enough for the example.
auto [oe_base, size] = Pal::reserve_at_least(bits::one_at_bit(28));
auto size = bits::one_at_bit(28);
auto oe_base = Pal::reserve(size);
Pal::notify_using<NoZero>(oe_base, size);
auto oe_end = pointer_offset(oe_base, size);
std::cout << "Allocated region " << oe_base << " - "
Expand Down
3 changes: 2 additions & 1 deletion src/test/func/pagemap/pagemap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ void test_pagemap(bool bounded)
// Initialise the pagemap
if (bounded)
{
auto [base, size] = Pal::reserve_at_least(bits::one_at_bit(30));
auto size = bits::one_at_bit(30);
auto base = Pal::reserve(size);
Pal::notify_using<NoZero>(base, size);
std::cout << "Fixed base: " << base << " (" << size << ") "
<< " end: " << pointer_offset(base, size) << std::endl;
Expand Down
2 changes: 2 additions & 0 deletions src/test/perf/external_pointer/externalpointer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ namespace test
size = 16;
// store object
objects[i] = (size_t*)alloc.alloc(size);
if (objects[i] == nullptr)
abort();
// Store allocators size for this object
*objects[i] = alloc.alloc_size(objects[i]);
}
Expand Down

0 comments on commit 5d0ae71

Please sign in to comment.