efficient / libcuckoo

A high-performance, concurrent hash table
Other
1.62k stars 275 forks source link

replace `std::aligned_storage` with `AlignedStorageType` #164

Closed JPewterschmidt closed 2 months ago

JPewterschmidt commented 2 months ago

Hi, since the std::aligned_storage has been deprecated in C++23 for some reason. I have re-implement it as AlignedStorageType to replace the STL one. For people who wanna use libcuckoo in C++23 without error message.

See also www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/n4907.pdf

ldalessa commented 2 months ago

The union type isn't being used properly either. You can't cast from the union through void* to storage_value_type* in places like storage_kvpair, you'd have to say &values_[ind].data_ there, and you need a std::launder too.

I would be a lot better to replace the aligned_storage with a union rather than perpetuating the char buffer.

using storage_value_type = std::pair<Key, T>;

union SlotType {
    storage_value_type value_;
    SlotType() {}
};

std::array<SlotType, SLOT_PER_BUCKET> values_;

const value_type &kvpair(size_type ind) const {
    // technically can't do this but it probably works fine
    // previous implementation does the same thing more-or-less
    return *reinterpret_cast<const value_type *>(&values_[ind].value_);
}
value_type &kvpair(size_type ind) {
    // technically can't do this but it probably works fine
    // previous implementation does the same thing more-or-less
    return *reinterpret_cast<value_type *>(&values_[ind].value_);
}

const storage_value_type &storage_kvpair(size_type ind) const {
    return values_[ind].value_;
}

storage_value_type &storage_kvpair(size_type ind) {
    return values_[ind].value_;
}
JPewterschmidt commented 2 months ago

Sorry for my wrong usage. Should I close this PR and open a new one or just leave it to you?

ldalessa commented 2 months ago

I'm not going to change anything. I think if you want to do the minimum change, you should just define your AlignedStorage the same way that cppreference defines aligned_storage.

template<std::size_t Len, std::size_t Align>
struct aligned_storage
{
    struct type
    {
        alignas(Align) unsigned char data[Len];
    };
};
JPewterschmidt commented 2 months ago

Alright then, thanks for your correction. I'll change it in my fork.