Skip to content

Commit

Permalink
slot_map: The free list should be LIFO, not FIFO.
Browse files Browse the repository at this point in the history 
This is mentioned at the end of Allan's paper
http://open-std.org/JTC1/SC22/WG21/docs/papers/2017/p0661r0.pdf

> Alternatively, the free list could store both a front and back index
> on the container, providing the ability to cycle through free slots
> more uniformly, reducing the chance of a generation counter overflow
> occurring. This is a quite valuable benefit with low overhead,
> so explicitly requiring it seems worthwhile.
  • Loading branch information
@Quuxplusone
Quuxplusone committed May 23, 2019
1 parent 459007f commit d47deb3
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 8 deletions.
43 changes: 35 additions & 8 deletions SG14/slot_map.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,14 +217,19 @@ class slot_map
// Functions for accessing and modifying the size of the slots container.
// These are beneficial as allocating more slots than values will cause the
// generation counter increases to be more evenly distributed across the slots.
// TODO [ajo]: The above comment is false, at least for this implementation.
//
constexpr void reserve_slots(size_type n) {
slot_map_detail::reserve_if_possible(slots_, n);
while (slots_.size() < n) {
auto idx = next_available_slot_index_;
next_available_slot_index_ = static_cast<key_index_type>(slots_.size());
slots_.emplace_back(key_type{idx, key_generation_type{}});
key_index_type original_num_slots = static_cast<key_index_type>(slots_.size());
if (original_num_slots < n) {
slots_.emplace_back(key_type{next_available_slot_index_, key_generation_type{}});
key_index_type last_new_slot = original_num_slots;
--n;
while (last_new_slot != n) {
slots_.emplace_back(key_type{last_new_slot, key_generation_type{}});
++last_new_slot;
}
next_available_slot_index_ = last_new_slot;
}
}
constexpr size_type slot_count() const { return slots_.size(); }
Expand All @@ -243,9 +248,15 @@ class slot_map
if (next_available_slot_index_ == slots_.size()) {
auto idx = next_available_slot_index_; ++idx;
slots_.emplace_back(key_type{idx, key_generation_type{}}); // make a new slot
last_available_slot_index_ = idx;
}
auto slot_iter = std::next(slots_.begin(), next_available_slot_index_);
next_available_slot_index_ = this->get_index(*slot_iter);
if (next_available_slot_index_ == last_available_slot_index_) {
next_available_slot_index_ = static_cast<key_index_type>(slots_.size());
last_available_slot_index_ = next_available_slot_index_;
} else {
next_available_slot_index_ = this->get_index(*slot_iter);
}
this->set_index(*slot_iter, value_pos);
key_type result = *slot_iter;
this->set_index(result, std::distance(slots_.begin(), slot_iter));
Expand Down Expand Up @@ -292,6 +303,7 @@ class slot_map
values_.clear();
reverse_map_.clear();
next_available_slot_index_ = key_index_type{};
last_available_slot_index_ = key_index_type{};
}

// swap is not mentioned in P0661r1 but it should be.
Expand All @@ -301,6 +313,7 @@ class slot_map
swap(values_, rhs.values_);
swap(reverse_map_, rhs.reverse_map_);
swap(next_available_slot_index_, rhs.next_available_slot_index_);
swap(last_available_slot_index_, rhs.last_available_slot_index_);
}

protected:
Expand Down Expand Up @@ -332,16 +345,30 @@ class slot_map
values_.pop_back();
reverse_map_.pop_back();
// Expire this key.
this->set_index(*slot_iter, next_available_slot_index_);
if (next_available_slot_index_ == slots_.size()) {
next_available_slot_index_ = static_cast<key_index_type>(slot_index);
last_available_slot_index_ = static_cast<key_index_type>(slot_index);
} else {
auto last_slot_iter = std::next(slots_.begin(), last_available_slot_index_);
this->set_index(*last_slot_iter, slot_index);
last_available_slot_index_ = static_cast<key_index_type>(slot_index);
}
this->increment_generation(*slot_iter);
next_available_slot_index_ = static_cast<key_index_type>(slot_index);
return std::next(values_.begin(), value_index);
}

Container<key_type> slots_; // high_water_mark() entries
Container<key_index_type> reverse_map_; // exactly size() entries
Container<mapped_type> values_; // exactly size() entries
key_index_type next_available_slot_index_{};
key_index_type last_available_slot_index_{};

// Class invariant:
// Either next_available_slot_index_ == last_available_slot_index_ == slots_.size(),
// or else 0 <= next_available_slot_index_ < slots_.size() and the "key" of that slot
// entry points to the subsequent available slot, and so on, until reaching
// last_available_slot_index_ (which might equal next_available_slot_index_ if there
// is only one available slot at the moment).
};

template<class T, class Key, template<class...> class Container>
Expand Down
43 changes: 43 additions & 0 deletions SG14_test/slot_map_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,42 @@ static void GenerationsDontSkipTest()
#endif
}

template<class SM>
static void IndexesAreUsedEvenlyTest()
{
using T = typename SM::mapped_type;
SM sm;
auto k1 = sm.emplace(Monad<T>::from_value(1));
auto k2 = sm.emplace(Monad<T>::from_value(2));
int original_cap = sm.slot_count();
for (int i=2; i < original_cap; ++i) {
sm.emplace(Monad<T>::from_value(i));
}
assert(sm.size() == sm.slot_count());

sm.erase(k1);
sm.erase(k2);
assert(sm.size() == sm.slot_count() - 2);

// There are now two slots available.
// So consecutive insertions should prefer to
// use different slots, rather than sticking to
// just one slot and bumping its generation count.
k1 = sm.emplace(Monad<T>::from_value(1));
sm.erase(k1);
k2 = sm.emplace(Monad<T>::from_value(2));
sm.erase(k2);

#if __cplusplus < 201703L
using std::get;
assert(get<0>(k2) != get<0>(k1));
#else
auto [idx1, gen1] = k1;
auto [idx2, gen2] = k2;
assert(idx2 != idx1);
#endif
}

void sg14_test::slot_map_test()
{
TypedefTests();
Expand All @@ -488,6 +524,7 @@ void sg14_test::slot_map_test()
ReserveTest<slot_map_1>();
VerifyCapacityExists<slot_map_1>(true);
GenerationsDontSkipTest<slot_map_1>();
IndexesAreUsedEvenlyTest<slot_map_1>();

// Test slot_map with a custom key type (C++14 destructuring).
using slot_map_2 = stdext::slot_map<unsigned long, TestKey::key_16_8_t>;
Expand All @@ -500,6 +537,7 @@ void sg14_test::slot_map_test()
ReserveTest<slot_map_2>();
VerifyCapacityExists<slot_map_2>(true);
GenerationsDontSkipTest<slot_map_2>();
IndexesAreUsedEvenlyTest<slot_map_2>();

#if __cplusplus >= 201703L
// Test slot_map with a custom key type (C++17 destructuring).
Expand All @@ -513,6 +551,7 @@ void sg14_test::slot_map_test()
ReserveTest<slot_map_3>();
VerifyCapacityExists<slot_map_3>(true);
GenerationsDontSkipTest<slot_map_3>();
IndexesAreUsedEvenlyTest<slot_map_3>();
#endif // __cplusplus >= 201703L

// Test slot_map with a custom (but standard and random-access) container type.
Expand All @@ -526,6 +565,7 @@ void sg14_test::slot_map_test()
ReserveTest<slot_map_4>();
VerifyCapacityExists<slot_map_4>(false);
GenerationsDontSkipTest<slot_map_4>();
IndexesAreUsedEvenlyTest<slot_map_4>();

// Test slot_map with a custom (non-standard, random-access) container type.
using slot_map_5 = stdext::slot_map<int, std::pair<unsigned, unsigned>, TestContainer::Vector>;
Expand All @@ -538,6 +578,7 @@ void sg14_test::slot_map_test()
ReserveTest<slot_map_5>();
VerifyCapacityExists<slot_map_5>(false);
GenerationsDontSkipTest<slot_map_5>();
IndexesAreUsedEvenlyTest<slot_map_5>();

// Test slot_map with a custom (standard, bidirectional-access) container type.
using slot_map_6 = stdext::slot_map<int, std::pair<unsigned, unsigned>, std::list>;
Expand All @@ -550,6 +591,7 @@ void sg14_test::slot_map_test()
ReserveTest<slot_map_6>();
VerifyCapacityExists<slot_map_6>(false);
GenerationsDontSkipTest<slot_map_6>();
IndexesAreUsedEvenlyTest<slot_map_6>();

// Test slot_map with a move-only value_type.
// Sadly, standard containers do not propagate move-only-ness, so we must use our custom Vector instead.
Expand All @@ -567,6 +609,7 @@ void sg14_test::slot_map_test()
ReserveTest<slot_map_7>();
VerifyCapacityExists<slot_map_7>(false);
GenerationsDontSkipTest<slot_map_7>();
IndexesAreUsedEvenlyTest<slot_map_7>();
}

#if defined(__cpp_concepts)
Expand Down

0 comments on commit d47deb3

Please sign in to comment.