diff --git a/xo-arena/include/xo/arena/DArenaHashMap.hpp b/xo-arena/include/xo/arena/DArenaHashMap.hpp index ee5fb794..50b7b4aa 100644 --- a/xo-arena/include/xo/arena/DArenaHashMap.hpp +++ b/xo-arena/include/xo/arena/DArenaHashMap.hpp @@ -67,12 +67,6 @@ namespace xo { size_type capacity() const noexcept { return store_.capacity(); } float load_factor() const noexcept { return store_.load_factor(); } - /** verify DArenaHashMap invariants - * Act on failure according to policy @p - * (combination of throw|log bits) - **/ - bool verify_ok(verify_policy p = verify_policy::throw_only()) const; - const_iterator cbegin() const { return this->_begin_aux(); } const_iterator cend() const { return this->_end_aux(); } @@ -111,6 +105,26 @@ namespace xo { /** establish kv pair for @p key in this table; return address of value part **/ mapped_type & operator[](const key_type & key); + /** verify DArenaHashMap invariants + * Act on failure according to policy @p + * (combination of throw|log bits) + **/ + bool verify_ok(verify_policy p = verify_policy::throw_only()) const; + + store_type * _store() noexcept { return &store_; } + + auto _hash(const key_type & key) const { + size_type h = hash_(key); + size_type h1 = h >> 7; // slot# + size_type h2 = h % 0x7f; // fingerprint + + size_type N = store_.capacity(); + + size_type slot_ix = h1 % (N - 1); + + return std::make_pair(slot_ix, h2); + } + private: iterator _promote_iterator(const_iterator ix) { return iterator(const_cast(ix._ctrl()), @@ -584,7 +598,9 @@ namespace xo { } /* SM1.3: n_group_ consistent with n_group_exponent_ */ - if (store_.n_group_ != (size_type{1} << store_.n_group_exponent_)) { + if ((store_.n_group_ > 0) + && (store_.n_group_ != (size_type{1} << store_.n_group_exponent_))) + { return policy.report_error(log, c_self, ": expect .n_group = 2^.n_group_exponent", xtag("n_group", store_.n_group_), @@ -655,15 +671,19 @@ namespace xo { } /* SM3.4: control_[stub+N+c_group_size+i] = c_iterator_bookend for i in [0, c_control_stub) */ - for (size_type i = 0; i < c_control_stub; ++i) { - size_type ix = c_control_stub + store_.n_slot_ + c_group_size + i; - if (store_.control_[ix] != c_iterator_bookend) { - return policy.report_error(log, - c_self, ": expect control_[stub+N+group+i] = c_iterator_bookend for end stub", - xtag("i", i), - xtag("ix", ix), - xtag("control_[ix]", (int)(store_.control_[ix])), - xtag("c_iterator_bookend", (int)c_iterator_bookend)); + if (store_.n_slot_ > 0) { + for (size_type i = 0; i < c_control_stub; ++i) { + size_type ix = c_control_stub + store_.n_slot_ + c_group_size + i; + if (store_.control_[ix] != c_iterator_bookend) { + return policy.report_error + (log, + c_self, ": expect control_[stub+N+group+i] = c_iterator_bookend for end stub", + xtag("i", i), + xtag("N", store_.n_slot_), + xtag("ix", ix), + xtag("control_[ix]", (int)(store_.control_[ix])), + xtag("c_iterator_bookend", (int)c_iterator_bookend)); + } } } diff --git a/xo-arena/include/xo/arena/hashmap/DArenaHashMapUtil.hpp b/xo-arena/include/xo/arena/hashmap/DArenaHashMapUtil.hpp index f2f72d17..372c99e0 100644 --- a/xo-arena/include/xo/arena/hashmap/DArenaHashMapUtil.hpp +++ b/xo-arena/include/xo/arena/hashmap/DArenaHashMapUtil.hpp @@ -49,7 +49,7 @@ namespace xo { using control_type = std::uint8_t; /** control: mask for sentinel states **/ - static constexpr uint8_t c_sentinel_mask = 0xF0; + static constexpr uint8_t c_sentinel_mask = 0x80; /** control: sentinel for empty slot **/ static constexpr uint8_t c_empty_slot = 0xFF; /** control: tombstone for deleted slot **/ @@ -82,7 +82,15 @@ namespace xo { /** control: compute size of control array for swiss hash map with @p n_slot cells **/ static constexpr size_type control_size(size_type n_slot) { - return n_slot + c_group_size + 2 * c_control_stub; + if (n_slot == 0) + return 0; + + /* control: + * - c_group_size overflow slots + * - 2x c_control_stub begin/end sentinels + */ + + return n_slot + c_group_size + (2 * c_control_stub); } /** find smallest multiple k : k * c_group_size >= n **/ diff --git a/xo-arena/include/xo/arena/hashmap/HashMapStore.hpp b/xo-arena/include/xo/arena/hashmap/HashMapStore.hpp index 661a6916..fd7c87f7 100644 --- a/xo-arena/include/xo/arena/hashmap/HashMapStore.hpp +++ b/xo-arena/include/xo/arena/hashmap/HashMapStore.hpp @@ -29,8 +29,14 @@ namespace xo { n_group_exponent_{group_exp2.first}, n_group_{group_exp2.second}, n_slot_{group_exp2.second * c_group_size}, - control_{control_vector_type::map(xo::mm::ArenaConfig{.name_ = name, .size_ = control_size(n_slot_)})}, - slots_{slot_vector_type::map(xo::mm::ArenaConfig{.name_ = name, .size_ = n_slot_ * sizeof(value_type)})} + control_{control_vector_type::map + (xo::mm::ArenaConfig{ + .name_ = name, + .size_ = control_size(n_slot_)})}, + slots_{slot_vector_type::map + (xo::mm::ArenaConfig{ + .name_ = name, + .size_ = n_slot_ * sizeof(value_type)})} { /* here: arenas have allocated address range, but no committed memory yet */ @@ -64,6 +70,7 @@ namespace xo { this->n_group_exponent_ = 0; this->n_group_ = 0; this->n_slot_ = 0; + this->control_.resize(0); this->slots_.resize(0); } diff --git a/xo-arena/utest/DArenaHashMap.test.cpp b/xo-arena/utest/DArenaHashMap.test.cpp index 0962def5..bc672c8b 100644 --- a/xo-arena/utest/DArenaHashMap.test.cpp +++ b/xo-arena/utest/DArenaHashMap.test.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include namespace xo { @@ -76,6 +77,8 @@ namespace xo { } REQUIRE(n == map.size()); } + + REQUIRE(map.verify_ok(verify_policy::chatty())); } { @@ -97,6 +100,8 @@ namespace xo { } REQUIRE(n == map.size()); } + + REQUIRE(map.verify_ok(verify_policy::chatty())); } { @@ -131,6 +136,8 @@ namespace xo { } REQUIRE(n == map.size()); } + + REQUIRE(map.verify_ok(verify_policy::chatty())); } { @@ -140,6 +147,8 @@ namespace xo { REQUIRE(map.size() == 0); REQUIRE(map.groups() == 0); REQUIRE(map.capacity() == 0); + + REQUIRE(map.verify_ok(verify_policy::chatty())); } /* slightly different starting point, 0 capacity! */ @@ -149,6 +158,8 @@ namespace xo { /* try_insert should fail - no capacity */ REQUIRE(!x.first); REQUIRE(!x.second); + + REQUIRE(map.verify_ok(verify_policy::chatty())); } { @@ -172,6 +183,8 @@ namespace xo { } REQUIRE(n == map.size()); } + + REQUIRE(map.verify_ok(verify_policy::chatty())); } } @@ -228,14 +241,27 @@ namespace xo { TEST_CASE("DArenaHashMap-operator-bracket", "[arena][DArenaHashMap]") { + scope log(XO_DEBUG(false)); + using HashMap = DArenaHashMap; HashMap map; + // copy keys here so we can print stuff + std::vector key_v; + // insert via operator[] map[1] = 100; + key_v.push_back(1); + REQUIRE(map.verify_ok(verify_policy::chatty())); + map[2] = 200; + key_v.push_back(2); + REQUIRE(map.verify_ok(verify_policy::chatty())); + map[3] = 300; + key_v.push_back(3); + REQUIRE(map.verify_ok(verify_policy::chatty())); REQUIRE(map.size() == 3); @@ -248,6 +274,7 @@ namespace xo { map[2] = 250; REQUIRE(map[2] == 250); REQUIRE(map.size() == 3); // size unchanged + REQUIRE(map.verify_ok(verify_policy::chatty())); // verify via find { @@ -265,9 +292,40 @@ namespace xo { REQUIRE(it != map.end()); REQUIRE(it->second == 300); } + { + auto it = map.find(4); + REQUIRE(it == map.end()); + } + + REQUIRE(map.verify_ok(verify_policy::chatty())); // operator[] on non-existent key creates default entry int & val = map[999]; + key_v.push_back(999); + + for (uint64_t i_slot = 0, N = map._store()->n_slot_; i_slot < N; ++i_slot) { + auto key = map._store()->slots_[i_slot].first; + auto ctrl = map._store()->control_ + [i_slot + DArenaHashMapUtil::c_control_stub]; + auto isdata = DArenaHashMapUtil::is_data(ctrl); + auto [h1,h2] = map._hash(key); + + if ((key != 0) + || (h1 != 0) + || (h2 != 0) + || (ctrl != DArenaHashMapUtil::c_empty_slot) + || isdata + ) { + log && log(xtag("i", i_slot), + xtag("key[i]", key), + xtag("h1", h1), xtag("h2", h2), + xtag("ctrl[i]", (int)ctrl), + xtag("isdata", isdata)); + } + } + + REQUIRE(map.verify_ok(verify_policy::chatty())); + REQUIRE(map.size() == 4); REQUIRE(val == 0); // default-initialized val = 999; @@ -280,6 +338,8 @@ namespace xo { HashMap map(1024); + REQUIRE(map.verify_ok()); + map["hello"] = 42; REQUIRE(map.size() == 1); REQUIRE(map.verify_ok());