Skip to content

Commit

Permalink
Allow hashtable to work with incomplete types.
Browse files Browse the repository at this point in the history
Experimentally verified using Ubuntu g++-5.4 and Apple clang-800.0.42.1.

The main issue is that some of our type_traits checks get instantiated in
compilation before the type is fully defined, which leads to a compilation
error. So we play some tricks to try and get them instantiated later.

1) declare operator<< and operator>> as friends to a generic KeyType and TType,
and use std::is_trivial on the function-specific templates. That way, the types
are instantiated after the class is defined.

2) Turn the is_simple and is_data_nothrow_move_constructible cosntexpr booleans
into constexpr methods. This seems to let them get instantiated later. This
also lets us use an incomplete key type, if so desired.

I'm not really sure if these fixes are future-proof, because they seem to rely
on compiler implementation details, and not some standards-documented behavior,
though I haven't really looked through the standard to verify.
  • Loading branch information
manugoyal committed Dec 24, 2018
1 parent 46e8289 commit cd2aa7a
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 17 deletions.
20 changes: 11 additions & 9 deletions libcuckoo/cuckoohash_map.hh
Original file line number Diff line number Diff line change
Expand Up @@ -725,13 +725,15 @@ private:

// true if the key is small and simple, which means using partial keys for
// lookup would probably slow us down
static constexpr bool is_simple =
std::is_pod<key_type>::value && sizeof(key_type) <= 8;
static constexpr bool is_simple() {
return std::is_pod<key_type>::value && sizeof(key_type) <= 8;
}

// Whether or not the data is nothrow-move-constructible.
static constexpr bool is_data_nothrow_move_constructible =
std::is_nothrow_move_constructible<key_type>::value &&
std::is_nothrow_move_constructible<mapped_type>::value;
static constexpr bool is_data_nothrow_move_constructible() {
return std::is_nothrow_move_constructible<key_type>::value &&
std::is_nothrow_move_constructible<mapped_type>::value;
}

// Contains a hash and partial for a given key. The partial key is used for
// partial-key cuckoohashing, and for finding the alternate bucket of that a
Expand Down Expand Up @@ -955,7 +957,7 @@ private:
spinlock &lock = locks[l];
if (lock.is_migrated()) return;

assert(is_data_nothrow_move_constructible);
assert(is_data_nothrow_move_constructible());
assert(locks.size() == kMaxNumLocks);
assert(old_buckets_.hashpower() + 1 == buckets_.hashpower());
assert(old_buckets_.size() >= kMaxNumLocks);
Expand Down Expand Up @@ -1164,7 +1166,7 @@ private:
// Silence a warning from MSVC about partial being unused if is_simple.
(void)partial;
for (int i = 0; i < static_cast<int>(slot_per_bucket()); ++i) {
if (!b.occupied(i) || (!is_simple && partial != b.partial(i))) {
if (!b.occupied(i) || (!is_simple() && partial != b.partial(i))) {
continue;
} else if (key_eq()(b.key(i), key)) {
return i;
Expand Down Expand Up @@ -1311,7 +1313,7 @@ private:
slot = -1;
for (int i = 0; i < static_cast<int>(slot_per_bucket()); ++i) {
if (b.occupied(i)) {
if (!is_simple && partial != b.partial(i)) {
if (!is_simple() && partial != b.partial(i)) {
continue;
}
if (key_eq()(b.key(i), key)) {
Expand Down Expand Up @@ -1673,7 +1675,7 @@ private:
// provides a strong exception guarantee.
template <typename TABLE_MODE, typename AUTO_RESIZE>
cuckoo_status cuckoo_fast_double(size_type current_hp) {
if (!is_data_nothrow_move_constructible) {
if (!is_data_nothrow_move_constructible()) {
LIBCUCKOO_DBG("%s", "cannot run cuckoo_fast_double because key-value"
" pair is not nothrow move constructible");
return cuckoo_expand_simple<TABLE_MODE, AUTO_RESIZE>(current_hp + 1);
Expand Down
20 changes: 12 additions & 8 deletions libcuckoo/libcuckoo_bucket_container.hh
Original file line number Diff line number Diff line change
Expand Up @@ -354,23 +354,27 @@ private:
// this should be okay. We could in theory just check if the type is
// TriviallyCopyable but this check is not available on some compilers we
// want to support.
template <typename Bogus = void *>
friend typename std::enable_if<sizeof(Bogus) && std::is_trivial<Key>::value &&
std::is_trivial<T>::value,
template <typename ThisKey, typename ThisT>
friend typename std::enable_if<std::is_trivial<ThisKey>::value &&
std::is_trivial<ThisT>::value,
std::ostream &>::type
operator<<(std::ostream &os, const libcuckoo_bucket_container &bc) {
operator<<(std::ostream &os,
const libcuckoo_bucket_container<ThisKey, ThisT, Allocator,
Partial, SLOT_PER_BUCKET> &bc) {
size_type hp = bc.hashpower();
os.write(reinterpret_cast<const char *>(&hp), sizeof(size_type));
os.write(reinterpret_cast<const char *>(bc.buckets_),
sizeof(bucket) * bc.size());
return os;
}

template <typename Bogus = void *>
friend typename std::enable_if<sizeof(Bogus) && std::is_trivial<Key>::value &&
std::is_trivial<T>::value,
template <typename ThisKey, typename ThisT>
friend typename std::enable_if<std::is_trivial<ThisKey>::value &&
std::is_trivial<ThisT>::value,
std::istream &>::type
operator>>(std::istream &is, libcuckoo_bucket_container &bc) {
operator>>(std::istream &is,
libcuckoo_bucket_container<ThisKey, ThisT, Allocator,
Partial, SLOT_PER_BUCKET> &bc) {
size_type hp;
is.read(reinterpret_cast<char *>(&hp), sizeof(size_type));
libcuckoo_bucket_container new_bc(hp, bc.get_allocator());
Expand Down

0 comments on commit cd2aa7a

Please sign in to comment.