Skip to content

Commit

Permalink
* gpage.h:
Browse files Browse the repository at this point in the history 
  * narrow the result of locations() *after* the division to raise the maximum page size by a few more bits. (Verify in the constructor that total_size / min_alloc <= INT_MAX so that locations() is guaranteed to be representable.)
  * inuse and starts both need to hold only one bit per location, not one bit per byte.
  * In the loop in allocate() that is searching for an available space, ensure that we do not misalign i when stepping over an allocated location at j.
  * In contains(), use std::less<> to compare pointers into potentially differing pages. (< is not portably a total function on pointers, std::less is portably total.) Avoid the same issue in deallocate by checking contains() before calculating the difference of the page base pointer and the function argument.
  * In lowest_hex_digits_of_address, convert the pointer to uintptr_t instead of size_t.
  * In debug_print, don't end lines with " \n" when "\n" will do ;)

* deferred_heap.h:
  * Convert the placement argument to void* to ensure we're invoking True Placement New in construct and construct_array.
  * In construct_array, construct the n elements in n contiguous memory locations instead of in a single memory location. If a constructor throws, cleanup the constructed elements before allowing the exception to propagate.
  * In collect, don't pass pg.page.locations() to pg.page.location_info() - it's out-of-range.

* test.cpp:
  * Tell the compiler not to warn that p1, p2, p4, & p5 are unused in test_page.
  * In test_deferred_allocator_set, test explicitly for libstdc++ and libc++ whose implementations of std::set do not like this test.
  • Loading branch information
@CaseyCarter
CaseyCarter committed Sep 27, 2016
1 parent 5f17581 commit 3a721c3
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 75 deletions.
21 changes: 14 additions & 7 deletions deferred_heap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ namespace gcpp {

// =====================================================================
// === BEGIN REENTRANCY-SAFE: ensure no in-progress use of private state
::new (p) T{ std::forward<Args>(args)... };
::new (static_cast<void*>(p.get())) T{ std::forward<Args>(args)... };
// === END REENTRANCY-SAFE: reload any stored copies of private state
// =====================================================================

Expand All @@ -898,7 +898,14 @@ namespace gcpp {
for (auto i = 0; i < n; ++i) {
// =====================================================================
// === BEGIN REENTRANCY-SAFE: ensure no in-progress use of private state
::new (p) T{};
try {
::new (static_cast<void*>(p.get() + i)) T{};
} catch(...) {
while (i-- > 0) {
(p.get() + i)->~T();
}
throw;
}
// === END REENTRANCY-SAFE: reload any stored copies of private state
// =====================================================================
}
Expand Down Expand Up @@ -1041,7 +1048,7 @@ namespace gcpp {

// find the end of the allocation
auto end_i = i + 1;
auto end = pg.page.location_info(pg.page.locations()).pointer;
auto end = [](auto s) { return s.data() + s.size(); }(pg.page.extent());
for (; end_i < pg.page.locations(); ++end_i) {
auto info = pg.page.location_info(end_i);
if (info.is_start) {
Expand All @@ -1051,7 +1058,7 @@ namespace gcpp {
}

// call the destructors for objects in this range
destroy_objects({ start.pointer, end - start.pointer });
destroy_objects({ start.pointer, end });

// and then deallocate the raw storage
pg.page.deallocate(start.pointer);
Expand All @@ -1062,7 +1069,7 @@ namespace gcpp {
// 5. finally, drop all now-unused pages
//
auto empty = pages.begin();
while ((empty = std::find_if(pages.begin(), pages.end(),
while ((empty = std::find_if(pages.begin(), pages.end(),
[](const auto& pg) { return pg.page.is_empty(); }))
!= pages.end()) {
Ensures(empty->deferred_ptrs.empty() && "page with no allocations still has deferred_ptrs");
Expand All @@ -1082,8 +1089,8 @@ namespace gcpp {
inline
void deferred_heap::debug_print() const
{
std::cout << "\n*** heap snapshot [" << (void*)this << "] *** "
<< pages.size() << " page" << (pages.size() != 1 ? "s *" : " **")
std::cout << "\n*** heap snapshot [" << (void*)this << "] *** "
<< pages.size() << " page" << (pages.size() != 1 ? "s *" : " **")
<< "***********************************\n\n";
for (auto& pg : pages) {
pg.page.debug_print();
Expand Down
96 changes: 58 additions & 38 deletions gpage.h
View file
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@

///////////////////////////////////////////////////////////////////////////////
//
// Copyright (c) 2016 Herb Sutter. All rights reserved.
//
// This code is licensed under the MIT License (MIT).
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
//
///////////////////////////////////////////////////////////////////////////////
//
// Copyright (c) 2016 Herb Sutter. All rights reserved.
//
// This code is licensed under the MIT License (MIT).
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
//
///////////////////////////////////////////////////////////////////////////////


#ifndef GCPP_GPAGE
#define GCPP_GPAGE

#include "bitflags.h"
#include "util.h"

#include <vector>
#include <algorithm>
Expand Down Expand Up @@ -62,13 +63,18 @@ namespace gcpp {
void operator=(gpage&) = delete;

public:
int locations() const noexcept { return gsl::narrow_cast<int>(total_size) / min_alloc; }
int locations() const noexcept { return gsl::narrow_cast<int>(total_size / min_alloc); }

const void* begin() const { return storage.get(); }
gsl::span<const byte> extent() const noexcept {
return { storage.get(), gsl::narrow_cast<std::ptrdiff_t>(total_size) };
}
gsl::span<byte> extent() noexcept {
return { storage.get(), gsl::narrow_cast<std::ptrdiff_t>(total_size) };
}

bool is_empty() const noexcept {
bool is_empty() const noexcept {
auto ret = inuse.all_false();
Ensures(!ret || starts.all_false() && "gpage with no inuse still has starts");
Ensures((!ret || starts.all_false()) && "gpage with no inuse still has starts");
return ret;
}

Expand Down Expand Up @@ -96,7 +102,7 @@ namespace gcpp {
std::size_t location;
std::size_t start_location;
};
contains_info_ret
contains_info_ret
contains_info(gsl::not_null<const byte*> p) const noexcept;

// Return whether there is an allocation starting at this location.
Expand Down Expand Up @@ -128,7 +134,7 @@ namespace gcpp {

// Construct a page with a given size and chunk size
//
inline
inline
gpage::gpage(std::size_t total_size_, std::size_t min_alloc_)
// total_size must be a multiple of min_alloc, so round up if necessary
: total_size(total_size_ +
Expand All @@ -137,11 +143,15 @@ namespace gcpp {
: 0))
, min_alloc(min_alloc_)
, storage(std::make_unique<byte[]>(total_size))
, inuse(total_size, false)
, starts(total_size, false)
, inuse(locations(), false)
, starts(locations(), false)
{
Expects(total_size % min_alloc == 0 &&
"total_size must be a multiple of min_alloc");
Expects(in_representable_range<int>(total_size / min_alloc) &&
"total_size / min_alloc must be representable by int");
Expects(in_representable_range<std::ptrdiff_t>(total_size) &&
"total_size must be representable by ptrdiff_t");
}


Expand All @@ -150,6 +160,9 @@ namespace gcpp {
template<class T>
byte* gpage::allocate(int n) noexcept {
Expects(n > 0 && "cannot request an empty allocation");
Expects(static_cast<std::size_t>(n) <=
std::numeric_limits<std::size_t>::max() / sizeof(T) &&
"sizeof(T)*n must be representable by std::size_t");

const auto bytes_needed = sizeof(T)*n;

Expand All @@ -160,7 +173,7 @@ namespace gcpp {

// check if we need to start at an offset from the beginning of the page
// because of alignment requirements, and also whether the request can fit
void* aligned_start = &storage[0];
void* aligned_start = storage.get();
auto aligned_space = total_size;
if (std::align(alignof(T), bytes_needed, aligned_start, aligned_space) == nullptr) {
return nullptr; // page can't have enough space for this #bytes, after alignment
Expand All @@ -170,16 +183,16 @@ namespace gcpp {
const auto locations_step = 1 + (alignof(T)-1) / min_alloc;

// # contiguous locations needed total
// note: as a simplification, for now we just add an extra location to every
// note: as a simplification, for now we just add an extra location to every
// allocation as a simple way to support one-past-the-end arithmetic
const auto locations_needed = (1 + (bytes_needed - 1) / min_alloc) + 1;

const auto end = locations() - locations_needed;
// intentionally omitting "+1" here in order to keep the
// intentionally omitting "+1" here in order to keep the
// last location valid for one-past-the-end pointing

// for each correctly aligned location candidate
std::size_t i = ((byte*)aligned_start - &storage[0]) / min_alloc;
std::size_t i = ((byte*)aligned_start - storage.get()) / min_alloc;
Expects(i == 0 && "temporary debug check: the current test harness shouldn't have generated something that required a starting offset for alignment reasons");
for (; i < end; i += locations_step) {
// check to see whether we have enough free locations starting here
Expand All @@ -189,7 +202,7 @@ namespace gcpp {
// if any location is in use, keep going
if (inuse.get(i + j)) {
// optimization: bump i to avoid probing the same location twice
i += j;
i += j - j % locations_step;
break;
}
}
Expand Down Expand Up @@ -222,16 +235,19 @@ namespace gcpp {
//
inline
bool gpage::contains(gsl::not_null<const byte*> p) const noexcept {
return &storage[0] <= p && p < &storage[total_size - 1];
// Use std::less<> to compare (possibly unrelated) pointers portably
auto const cmp = std::less<>{};
auto const ext = extent();
return !cmp(p, ext.data()) && cmp(p, ext.data() + ext.size());
}

inline
gpage::contains_info_ret gpage::contains_info(gsl::not_null<const byte*> p) const noexcept {
if (!(&storage[0] <= p && p < &storage[total_size - 1])) {
if (!contains(p)) {
return{ not_in_range, 0, 0 };
}

auto where = (p - &storage[0]) / min_alloc;
auto where = (p - storage.get()) / min_alloc;
if (!inuse.get(where)) {
return{ in_range_unallocated, where, 0 };
}
Expand All @@ -252,7 +268,7 @@ namespace gcpp {

// Return whether there is an allocation starting at this location.
//
inline
inline
gpage::location_info_ret
gpage::location_info(int where) const noexcept {
return{ starts.get(where), &storage[where*min_alloc] };
Expand All @@ -263,11 +279,13 @@ namespace gcpp {
//
inline
void gpage::deallocate(gsl::not_null<byte*> p) noexcept {
auto here = gsl::narrow_cast<int>((p - &storage[0]) / min_alloc);
// p had better point to our storage ...
Expects(contains(p) && "attempt to deallocate - out of range");

auto here = gsl::narrow_cast<int>((p - storage.get()) / min_alloc);

// p had better point to our storage and to the start of an allocation
// ... and to the start of an allocation
// (note: we could also check alignment here but that seems superfluous)
Expects(0 <= here && here < locations() && "attempt to deallocate - out of range");
Expects(starts.get(here) && "attempt to deallocate - not at start of a valid allocation");
Expects(inuse.get(here) && "attempt to deallocate - location is not in use");

Expand Down Expand Up @@ -307,7 +325,7 @@ namespace gcpp {
static const char digits[] = "0123456789ABCDEF";

std::string ret(num, ' ');
std::size_t val = (std::size_t)p;
auto val = reinterpret_cast<std::uintptr_t>(p);
while (num-- > 0) {
ret[num] = digits[val % 16];
val >>= 4;
Expand All @@ -317,7 +335,7 @@ namespace gcpp {

inline
void gpage::debug_print() const {
auto base = &storage[0];
auto base = storage.get();
std::cout << "--- total_size " << total_size << " --- min_alloc " << min_alloc
<< " --- " << (void*)base << " ---------------------------\n ";

Expand All @@ -335,8 +353,10 @@ namespace gcpp {
for (auto i = 0; i < locations(); ++i) {
if (i % 64 == 0) { std::cout << lowest_hex_digits_of_address(base + i*min_alloc, 4) << ' '; }
std::cout << (starts.get(i) ? 'A' : inuse.get(i) ? 'a' : '.');
if (i % 8 == 7) { std::cout << ' '; }
if (i % 64 == 63) { std::cout << '\n'; }
if (i % 8 == 7) {
if (i % 64 == 63) { std::cout << '\n'; }
else { std::cout << ' '; }
}
}

std::cout << '\n';
Expand Down
34 changes: 19 additions & 15 deletions test.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@

///////////////////////////////////////////////////////////////////////////////
//
// Copyright (c) 2016 Herb Sutter. All rights reserved.
//
// This code is licensed under the MIT License (MIT).
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
//
///////////////////////////////////////////////////////////////////////////////
//
// Copyright (c) 2016 Herb Sutter. All rights reserved.
//
// This code is licensed under the MIT License (MIT).
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
//
///////////////////////////////////////////////////////////////////////////////


Expand Down Expand Up @@ -84,21 +84,25 @@ void test_page() {
g.debug_print();

auto p1 = g.allocate<char>();
(void)p1;
g.debug_print();

auto p2 = g.allocate<double>();
(void)p2;
g.debug_print();

auto p3 = g.allocate<char>();
g.debug_print();

auto p4 = g.allocate<double>();
(void)p4;
g.debug_print();

g.deallocate(p3);
g.debug_print();

auto p5 = g.allocate<char>();
(void)p5;
g.debug_print();
}

Expand Down Expand Up @@ -227,7 +231,7 @@ void test_deferred_allocator() {
//----------------------------------------------------------------------------

void test_deferred_allocator_set() {
#ifndef __GNUC__
#if !defined(__GLIBCXX__) && !defined(_LIBCPP_VERSION)
deferred_heap heap;

auto s = deferred_set<widget>(heap);
Expand Down
Loading

0 comments on commit 3a721c3

Please sign in to comment.