Skip to content

Commit 3c41bcc

Browse files
author
Julian LALU
committed
Improve hashset copy constructor for bitwise copy constructible type
1 parent 031a07a commit 3c41bcc

File tree

2 files changed

+57
-80
lines changed

2 files changed

+57
-80
lines changed

‎interface/core/containers/hashset.h‎

Lines changed: 6 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ namespace hud
596596
// In a non constant evaluated context
597597
// If type is trivially copy constructible, just memcpy control and slot
598598
// else do like grow_capacity
599-
if (hud::is_constant_evaluated() || hud::is_trivially_copy_constructible_v<slot_type>)
599+
if (hud::is_constant_evaluated() || !hud::is_bitwise_copy_constructible_v<slot_type>)
600600
{
601601
// Set control to empty ending with sentinel
602602
hud::memory::set(control_ptr_, control_size, empty_byte);
@@ -616,41 +616,12 @@ namespace hud
616616
hud::memory::construct_at(slot_ptr_ + slot_index, slot);
617617
}
618618
}
619+
else
620+
{
621+
hud::memory::copy_construct_array(control_ptr_, other.control_ptr_, control_size_for_max_count(max_slot_count_));
622+
hud::memory::copy_construct_array(slot_ptr_, other.slot_ptr_, max_slot_count_);
623+
}
619624
}
620-
// Set control to empty ending with sentinel only if type is not trivially copyable
621-
// Int he case of trivially copyable type we just memcpy control and slots
622-
// if (!hud::is_trivially_copy_constructible_v<slot_type>)
623-
// {
624-
// hud::memory::set(control_ptr_, control_size, empty_byte);
625-
// control_ptr_[max_slot_count_] = sentinel_byte;
626-
// }
627-
628-
// If we have elements, insert them to the new buffer
629-
// if (other.count() > 0)
630-
// {
631-
// if (hud::is_trivially_copy_constructible_v<slot_type>)
632-
// {
633-
// hud::memory::copy_construct_array(control_ptr_, other.control_ptr_, other.max_slot_count_);
634-
// hud::memory::copy_construct_array(slot_ptr_, other.slot_ptr_, other.max_slot_count_);
635-
// }
636-
// else
637-
// {
638-
// // Move elements to new buffer if any
639-
// // Relocate slots to newly allocated buffer
640-
// for (auto &slot : other)
641-
// {
642-
// // Compute the hash
643-
// u64 hash = hasher_type {}(slot.key());
644-
// // Find H1 slot index
645-
// u64 h1 = H1(hash);
646-
// usize slot_index = find_first_empty_or_deleted(control_ptr_, max_slot_count_, h1);
647-
// // Save h2 in control h1 index
648-
// control::set_h2(control_ptr_, slot_index, H2(hash), max_slot_count_);
649-
// // Copy slot
650-
// hud::memory::construct_at(slot_ptr_ + slot_index, slot);
651-
// }
652-
// }
653-
// }
654625
}
655626

656627
constexpr ~hashset_impl() noexcept

‎test/hashmap/hashmap_copy_constructors.cpp‎

Lines changed: 51 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ GTEST_TEST(hashmap, copy_construct_bitwise_copy_constructible_same_type_same_all
2424
using AllocatorType = hud_test::allocator_watcher<1>;
2525
using NewType = hud::hashmap<key_type, value_type, hud::hashmap_default_hasher<key_type>, hud::hashmap_default_key_equal<key_type>, AllocatorType>;
2626
using CopiedType = hud::hashmap<key_type, value_type, hud::hashmap_default_hasher<key_type>, hud::hashmap_default_key_equal<key_type>, AllocatorType>;
27-
27+
static_assert(hud::is_trivially_copy_constructible_v<CopiedType::slot_type>);
2828
// No extra
2929
{
3030
auto test_default_allocator = [](std::initializer_list<hud::pair<i32, i64>> initializer)
@@ -101,72 +101,78 @@ GTEST_TEST(hashmap, copy_construct_bitwise_copy_constructible_same_type_same_all
101101
hud_assert_true(std::get<4>(result));
102102
}
103103

104-
// auto test_with_allocator = [](std::initializer_list<i32> initializer, usize copied_extra)
104+
// auto test_with_allocator = [](std::initializer_list<hud::pair<i32, i64>> initializer)
105105
// {
106-
// const CopiedType copied(initializer, copied_extra);
106+
// const CopiedType copied(initializer);
107107

108-
// // Copy the array
109-
// hud::array<type, AllocatorType> copy(copied, AllocatorType {});
108+
// // Copy the map
109+
// NewType copy(copied, AllocatorType {});
110110

111-
// // Ensure we copy all datas in order
112-
// bool all_values_copied = true;
111+
// // Ensure we copy all elements
112+
// bool all_keys_and_values_copied = true;
113113
// for (usize index = 0; index < initializer.size(); index++)
114114
// {
115-
// if (copy[index] != static_cast<type>(index))
115+
// const auto &init_elem = (initializer.begin() + index);
116+
// const auto it = copy.find(init_elem->first);
117+
// if (it == copy.end())
118+
// {
119+
// all_keys_and_values_copied = false;
120+
// break;
121+
// }
122+
// if (it->key() != init_elem->first)
123+
// {
124+
// all_keys_and_values_copied = false;
125+
// break;
126+
// }
127+
// if (it->value() != init_elem->second)
116128
// {
117-
// all_values_copied = false;
129+
// all_keys_and_values_copied = false;
118130
// break;
119131
// }
120132
// }
121133

122134
// return std::tuple {
123-
// // Ensure we copy all datas in order
124-
// copy.data() != nullptr,
125-
// copy.count(),
126-
// copy.max_count(),
127-
// all_values_copied,
128-
129-
// // Ensure the copy data is not the same memory of the copied data
130-
// copied.data() != copy.data(),
131-
132-
// // Ensure we are allocating only one time
133-
// copy.allocator().allocation_count(),
134-
// copy.allocator().free_count()
135+
// copy.count() == copied.count(), // 0
136+
// copy.max_count() == copied.max_count(), // 1
137+
// all_keys_and_values_copied, // 2
138+
// copy.allocator().allocation_count() == (initializer.size() > 0 ? (hud::is_constant_evaluated() ? 4 : 2) : 0), // 3
139+
// copy.allocator().free_count() == 0 // 4
135140
// };
136141
// };
137142

138143
// // Non constant
139144
// {
140-
// const auto result = test_with_allocator({0, 1, 2, 3}, 1u);
141-
// // Ensure we copy all datas in order
142-
// hud_assert_true(std::get<0>(result));
143-
// hud_assert_eq(std::get<1>(result), 4u);
144-
// hud_assert_eq(std::get<2>(result), 4u + 1u);
145-
// hud_assert_true(std::get<3>(result));
146-
147-
// // Ensure the copy data is not the same memory of the copied data
145+
// const auto result_empty = test_with_allocator({});
146+
// hud_assert_true(std::get<0>(result_empty));
147+
// hud_assert_true(std::get<1>(result_empty));
148+
// hud_assert_true(std::get<2>(result_empty));
149+
// hud_assert_true(std::get<3>(result_empty));
150+
// hud_assert_true(std::get<4>(result_empty));
151+
152+
// const auto result = test_with_allocator(TEST_VALUES);
153+
// hud_assert_true(std::get<0>(result));
154+
// hud_assert_true(std::get<1>(result));
155+
// hud_assert_true(std::get<2>(result));
156+
// hud_assert_true(std::get<3>(result));
148157
// hud_assert_true(std::get<4>(result));
149-
150-
// // Ensure we are allocating only one time
151-
// hud_assert_eq(std::get<5>(result), 1u);
152-
// hud_assert_eq(std::get<6>(result), 0u);
153158
// }
154159

155160
// // Constant
156161
// {
157-
// constexpr auto result = test_with_allocator({0, 1, 2, 3}, 1u);
158-
// // Ensure we copy all datas in order
159-
// hud_assert_true(std::get<0>(result));
160-
// hud_assert_eq(std::get<1>(result), 4u);
161-
// hud_assert_eq(std::get<2>(result), 4u + 1u);
162-
// hud_assert_true(std::get<3>(result));
163-
164-
// // Ensure the copy data is not the same memory of the copied data
165-
// hud_assert_true(std::get<4>(result));
166162

167-
// // Ensure we are allocating only one time
168-
// hud_assert_eq(std::get<5>(result), 1u);
169-
// hud_assert_eq(std::get<6>(result), 0u);
163+
// constexpr auto result_empty = test_with_allocator({});
164+
// hud_assert_true(std::get<0>(result_empty));
165+
// hud_assert_true(std::get<1>(result_empty));
166+
// hud_assert_true(std::get<2>(result_empty));
167+
// hud_assert_true(std::get<3>(result_empty));
168+
// hud_assert_true(std::get<4>(result_empty));
169+
170+
// constexpr auto result = test_with_allocator(TEST_VALUES);
171+
// hud_assert_true(std::get<0>(result));
172+
// hud_assert_true(std::get<1>(result));
173+
// hud_assert_true(std::get<2>(result));
174+
// hud_assert_true(std::get<3>(result));
175+
// hud_assert_true(std::get<4>(result));
170176
// }
171177
}
172178

0 commit comments

Comments
 (0)