From 45edb98cd982cd3be7b66139b9016bea8d46bf56 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Sun, 30 Apr 2023 11:00:37 -0400 Subject: [PATCH] Replace custom `CompressedTuple` with `std::tuple`. --- CMakeLists.txt | 3 - parallel_hashmap/btree.h | 20 ++-- parallel_hashmap/phmap.h | 19 ++-- parallel_hashmap/phmap_base.h | 174 ---------------------------- tests/compressed_tuple_test.cc | 201 --------------------------------- 5 files changed, 17 insertions(+), 400 deletions(-) delete mode 100644 tests/compressed_tuple_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 7b0a6ea..db01352 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -107,9 +107,6 @@ if (PHMAP_BUILD_TESTS) enable_testing() ## ---------------- regular hash maps ---------------------------- - phmap_cc_test(NAME compressed_tuple SRCS "tests/compressed_tuple_test.cc" - DEPS ${PHMAP_GTEST_LIBS}) - phmap_cc_test(NAME container_memory SRCS "tests/container_memory_test.cc" DEPS ${PHMAP_GTEST_LIBS}) diff --git a/parallel_hashmap/btree.h b/parallel_hashmap/btree.h index 5304e42..0cafd18 100644 --- a/parallel_hashmap/btree.h +++ b/parallel_hashmap/btree.h @@ -1861,7 +1861,7 @@ namespace priv { void swap(btree &x); const key_compare &key_comp() const noexcept { - return root_.template get<0>(); + return std::get<0>(root_); } template bool compare_keys(const K &x, const LK &y) const { @@ -1954,10 +1954,10 @@ namespace priv { private: // Internal accessor routines. - node_type *root() { return root_.template get<2>(); } - const node_type *root() const { return root_.template get<2>(); } - node_type *&mutable_root() noexcept { return root_.template get<2>(); } - key_compare *mutable_key_comp() noexcept { return &root_.template get<0>(); } + node_type *root() { return std::get<2>(root_); } + const node_type *root() const { return std::get<2>(root_); } + node_type *&mutable_root() noexcept { return std::get<2>(root_); } + key_compare *mutable_key_comp() noexcept { return &std::get<0>(root_); } // The leftmost node is stored as the parent of the root node. node_type *leftmost() { return root()->parent(); } @@ -1965,10 +1965,10 @@ namespace priv { // Allocator routines. allocator_type *mutable_allocator() noexcept { - return &root_.template get<1>(); + return &std::get<1>(root_); } const allocator_type &allocator() const noexcept { - return root_.template get<1>(); + return std::get<1>(root_); } // Allocates a correctly aligned node of at least size bytes using the @@ -2110,11 +2110,7 @@ namespace priv { } private: - // We use compressed tuple in order to save space because key_compare and - // allocator_type are usually empty. - phmap::priv::CompressedTuple - root_; + std::tuple root_; // A pointer to the rightmost node. Note that the leftmost node is stored as // the root's parent. diff --git a/parallel_hashmap/phmap.h b/parallel_hashmap/phmap.h index 37f0f7c..15a92a4 100644 --- a/parallel_hashmap/phmap.h +++ b/parallel_hashmap/phmap.h @@ -2281,7 +2281,7 @@ private: growth_left() = CapacityToGrowth(capacity) - size_; } - size_t& growth_left() { return settings_.template get<0>(); } + size_t& growth_left() { return std::get<0>(settings_); } template class RefSet, @@ -2309,13 +2309,13 @@ private: // small tables. bool is_small() const { return capacity_ < Group::kWidth - 1; } - hasher& hash_ref() { return settings_.template get<1>(); } - const hasher& hash_ref() const { return settings_.template get<1>(); } - key_equal& eq_ref() { return settings_.template get<2>(); } - const key_equal& eq_ref() const { return settings_.template get<2>(); } - allocator_type& alloc_ref() { return settings_.template get<3>(); } + hasher& hash_ref() { return std::get<1>(settings_); } + const hasher& hash_ref() const { return std::get<1>(settings_); } + key_equal& eq_ref() { return std::get<2>(settings_); } + const key_equal& eq_ref() const { return std::get<2>(settings_); } + allocator_type& alloc_ref() { return std::get<3>(settings_); } const allocator_type& alloc_ref() const { - return settings_.template get<3>(); + return std::get<3>(settings_); } // TODO(alkis): Investigate removing some of these fields: @@ -2326,9 +2326,8 @@ private: size_t size_ = 0; // number of full slots size_t capacity_ = 0; // total number of slots HashtablezInfoHandle infoz_; - phmap::priv::CompressedTuple - settings_{0, hasher{}, key_equal{}, allocator_type{}}; + std::tuple + settings_{0, hasher{}, key_equal{}, allocator_type{}}; }; diff --git a/parallel_hashmap/phmap_base.h b/parallel_hashmap/phmap_base.h index 28676d5..d3b816b 100644 --- a/parallel_hashmap/phmap_base.h +++ b/parallel_hashmap/phmap_base.h @@ -4127,180 +4127,6 @@ public: : internal_layout::LayoutType(sizes...) {} }; -} // namespace priv -} // namespace phmap - -// --------------------------------------------------------------------------- -// compressed_tuple.h -// --------------------------------------------------------------------------- - -#ifdef _MSC_VER - // We need to mark these classes with this declspec to ensure that - // CompressedTuple happens. - #define PHMAP_INTERNAL_COMPRESSED_TUPLE_DECLSPEC __declspec(empty_bases) -#else // _MSC_VER - #define PHMAP_INTERNAL_COMPRESSED_TUPLE_DECLSPEC -#endif // _MSC_VER - -namespace phmap { -namespace priv { - -template -class CompressedTuple; - -namespace internal_compressed_tuple { - -template -struct Elem; -template -struct Elem, I> - : std::tuple_element> {}; -template -using ElemT = typename Elem::type; - -// --------------------------------------------------------------------------- -// Use the __is_final intrinsic if available. Where it's not available, classes -// declared with the 'final' specifier cannot be used as CompressedTuple -// elements. -// TODO(sbenza): Replace this with std::is_final in C++14. -// --------------------------------------------------------------------------- -template -constexpr bool IsFinal() { -#if defined(__clang__) || defined(__GNUC__) - return __is_final(T); -#else - return false; -#endif -} - -template -constexpr bool ShouldUseBase() { -#ifdef __INTEL_COMPILER - // avoid crash in Intel compiler - // assertion failed at: "shared/cfe/edgcpfe/lower_init.c", line 7013 - return false; -#else - return std::is_class::value && std::is_empty::value && !IsFinal(); -#endif -} - -// The storage class provides two specializations: -// - For empty classes, it stores T as a base class. -// - For everything else, it stores T as a member. -// ------------------------------------------------ -template >()> -struct Storage -{ - using T = ElemT; - T value; - constexpr Storage() = default; - explicit constexpr Storage(T&& v) : value(phmap::forward(v)) {} - constexpr const T& get() const& { return value; } - T& get() & { return value; } - constexpr const T&& get() const&& { return phmap::move(*this).value; } - T&& get() && { return std::move(*this).value; } -}; - -template -struct PHMAP_INTERNAL_COMPRESSED_TUPLE_DECLSPEC Storage - : ElemT -{ - using T = internal_compressed_tuple::ElemT; - constexpr Storage() = default; - explicit constexpr Storage(T&& v) : T(phmap::forward(v)) {} - constexpr const T& get() const& { return *this; } - T& get() & { return *this; } - constexpr const T&& get() const&& { return phmap::move(*this); } - T&& get() && { return std::move(*this); } -}; - -template -struct PHMAP_INTERNAL_COMPRESSED_TUPLE_DECLSPEC CompressedTupleImpl; - -template -struct PHMAP_INTERNAL_COMPRESSED_TUPLE_DECLSPEC - CompressedTupleImpl, phmap::index_sequence> - // We use the dummy identity function through std::integral_constant to - // convince MSVC of accepting and expanding I in that context. Without it - // you would get: - // error C3548: 'I': parameter pack cannot be used in this context - : Storage, - std::integral_constant::value>... -{ - constexpr CompressedTupleImpl() = default; - explicit constexpr CompressedTupleImpl(Ts&&... args) - : Storage, I>(phmap::forward(args))... {} -}; - -} // namespace internal_compressed_tuple - -// --------------------------------------------------------------------------- -// Helper class to perform the Empty Base Class Optimization. -// Ts can contain classes and non-classes, empty or not. For the ones that -// are empty classes, we perform the CompressedTuple. If all types in Ts are -// empty classes, then CompressedTuple is itself an empty class. -// -// To access the members, use member .get() function. -// -// Eg: -// phmap::priv::CompressedTuple value(7, t1, t2, -// t3); -// assert(value.get<0>() == 7); -// T1& t1 = value.get<1>(); -// const T2& t2 = value.get<2>(); -// ... -// -// https://en.cppreference.com/w/cpp/language/ebo -// --------------------------------------------------------------------------- -template -class PHMAP_INTERNAL_COMPRESSED_TUPLE_DECLSPEC CompressedTuple - : private internal_compressed_tuple::CompressedTupleImpl< - CompressedTuple, phmap::index_sequence_for> -{ -private: - template - using ElemT = internal_compressed_tuple::ElemT; - -public: - constexpr CompressedTuple() = default; - explicit constexpr CompressedTuple(Ts... base) - : CompressedTuple::CompressedTupleImpl(phmap::forward(base)...) {} - - template - ElemT& get() & { - return internal_compressed_tuple::Storage::get(); - } - - template - constexpr const ElemT& get() const& { - return internal_compressed_tuple::Storage::get(); - } - - template - ElemT&& get() && { - return std::move(*this) - .internal_compressed_tuple::template Storage::get(); - } - - template - constexpr const ElemT&& get() const&& { - return phmap::move(*this) - .internal_compressed_tuple::template Storage::get(); - } -}; - -// Explicit specialization for a zero-element tuple -// (needed to avoid ambiguous overloads for the default constructor). -// --------------------------------------------------------------------------- -template <> -class PHMAP_INTERNAL_COMPRESSED_TUPLE_DECLSPEC CompressedTuple<> {}; - -} // namespace priv -} // namespace phmap - - -namespace phmap { -namespace priv { #ifdef _MSC_VER #pragma warning(push) diff --git a/tests/compressed_tuple_test.cc b/tests/compressed_tuple_test.cc deleted file mode 100644 index f73b5e2..0000000 --- a/tests/compressed_tuple_test.cc +++ /dev/null @@ -1,201 +0,0 @@ -// Copyright 2018 The Abseil Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "parallel_hashmap/phmap.h" - -#include -#include - -#include "gmock/gmock.h" -#include "gtest/gtest.h" - -namespace phmap { -namespace priv { -namespace { - -enum class CallType { kConstRef, kConstMove }; - -template -struct Empty { - constexpr CallType value() const& { return CallType::kConstRef; } - constexpr CallType value() const&& { return CallType::kConstMove; } -}; - -template -struct NotEmpty { - T value; -}; - -template -struct TwoValues { - T value1; - U value2; -}; - -TEST(CompressedTupleTest, Sizeof) { - EXPECT_EQ(sizeof(int), sizeof(CompressedTuple)); - EXPECT_EQ(sizeof(int), sizeof(CompressedTuple>)); - EXPECT_EQ(sizeof(int), sizeof(CompressedTuple, Empty<1>>)); - EXPECT_EQ(sizeof(int), - sizeof(CompressedTuple, Empty<1>, Empty<2>>)); - - EXPECT_EQ(sizeof(TwoValues), - sizeof(CompressedTuple>)); - EXPECT_EQ(sizeof(TwoValues), - sizeof(CompressedTuple, NotEmpty>)); - EXPECT_EQ(sizeof(TwoValues), - sizeof(CompressedTuple, NotEmpty, Empty<1>>)); -} - -TEST(CompressedTupleTest, Access) { - struct S { - std::string x; - }; - CompressedTuple, S> x(7, {}, S{"ABC"}); - EXPECT_EQ(sizeof(x), sizeof(TwoValues)); - EXPECT_EQ(7, x.get<0>()); - EXPECT_EQ("ABC", x.get<2>().x); -} - -TEST(CompressedTupleTest, NonClasses) { - CompressedTuple x(7, "ABC"); - EXPECT_EQ(7, x.get<0>()); - EXPECT_STREQ("ABC", x.get<1>()); -} - -TEST(CompressedTupleTest, MixClassAndNonClass) { - CompressedTuple, NotEmpty> x(7, "ABC", {}, - {1.25}); - struct Mock { - int v; - const char* p; - double d; - }; - EXPECT_EQ(sizeof(x), sizeof(Mock)); - EXPECT_EQ(7, x.get<0>()); - EXPECT_STREQ("ABC", x.get<1>()); - EXPECT_EQ(1.25, x.get<3>().value); -} - -TEST(CompressedTupleTest, Nested) { - CompressedTuple, - CompressedTuple>> - x(1, CompressedTuple(2), - CompressedTuple>(3, CompressedTuple(4))); - EXPECT_EQ(1, x.get<0>()); - EXPECT_EQ(2, x.get<1>().get<0>()); - EXPECT_EQ(3, x.get<2>().get<0>()); - EXPECT_EQ(4, x.get<2>().get<1>().get<0>()); - - CompressedTuple, Empty<0>, - CompressedTuple, CompressedTuple>>> - y; - std::set*> empties{&y.get<0>(), &y.get<1>(), &y.get<2>().get<0>(), - &y.get<2>().get<1>().get<0>()}; -#ifdef _MSC_VER - // MSVC has a bug where many instances of the same base class are layed out in - // the same address when using __declspec(empty_bases). - // This will be fixed in a future version of MSVC. - int expected = 1; -#else - int expected = 4; -#endif - EXPECT_EQ(expected, sizeof(y)); - EXPECT_EQ(expected, empties.size()); - EXPECT_EQ(sizeof(y), sizeof(Empty<0>) * empties.size()); - - EXPECT_EQ(4 * sizeof(char), - sizeof(CompressedTuple, - CompressedTuple>)); - EXPECT_TRUE( - (std::is_empty>, - CompressedTuple>>>::value)); -} - -TEST(CompressedTupleTest, Reference) { - int i = 7; - std::string s = "Very long std::string that goes in the heap"; - CompressedTuple x(i, i, s, s); - - // Sanity check. We should have not moved from `s` - EXPECT_EQ(s, "Very long std::string that goes in the heap"); - - EXPECT_EQ(x.get<0>(), x.get<1>()); - EXPECT_NE(&x.get<0>(), &x.get<1>()); - EXPECT_EQ(&x.get<1>(), &i); - - EXPECT_EQ(x.get<2>(), x.get<3>()); - EXPECT_NE(&x.get<2>(), &x.get<3>()); - EXPECT_EQ(&x.get<3>(), &s); -} - -TEST(CompressedTupleTest, NoElements) { - CompressedTuple<> x; - static_cast(x); // Silence -Wunused-variable. - EXPECT_TRUE(std::is_empty>::value); -} - -TEST(CompressedTupleTest, MoveOnlyElements) { - CompressedTuple> str_tup( - phmap::make_unique("str")); - - CompressedTuple>, - std::unique_ptr> - x(std::move(str_tup), phmap::make_unique(5)); - - EXPECT_EQ(*x.get<0>().get<0>(), "str"); - EXPECT_EQ(*x.get<1>(), 5); - - std::unique_ptr x0 = std::move(x.get<0>()).get<0>(); - std::unique_ptr x1 = std::move(x).get<1>(); - - EXPECT_EQ(*x0, "str"); - EXPECT_EQ(*x1, 5); -} - -TEST(CompressedTupleTest, Constexpr) { - constexpr CompressedTuple, Empty<0>> x( - 7, 1.25, CompressedTuple(5), {}); - constexpr int x0 = x.get<0>(); - constexpr double x1 = x.get<1>(); - constexpr int x2 = x.get<2>().get<0>(); - constexpr CallType x3 = x.get<3>().value(); - - EXPECT_EQ(x0, 7); - EXPECT_EQ(x1, 1.25); - EXPECT_EQ(x2, 5); - EXPECT_EQ(x3, CallType::kConstRef); - -#if defined(__clang__) - // An apparent bug in earlier versions of gcc claims these are ambiguous. - constexpr int x2m = std::move(x.get<2>()).get<0>(); - constexpr CallType x3m = std::move(x).get<3>().value(); - EXPECT_EQ(x2m, 5); - EXPECT_EQ(x3m, CallType::kConstMove); -#endif -} - -#if defined(__clang__) || defined(__GNUC__) -TEST(CompressedTupleTest, EmptyFinalClass) { - struct S final { - int f() const { return 5; } - }; - CompressedTuple x; - EXPECT_EQ(x.get<0>().f(), 5); -} -#endif - -} // namespace -} // namespace priv -} // namespace phmap