diff --git a/hwy/contrib/btree/btree-inl.h b/hwy/contrib/btree/btree-inl.h index 5d9827694c..498a33a409 100644 --- a/hwy/contrib/btree/btree-inl.h +++ b/hwy/contrib/btree/btree-inl.h @@ -1442,6 +1442,14 @@ void MergeLeaves(MapLeafNode* leaf, // 2. Adapter Mode (Non-owning): Constructed with an external BTreeState pointer // (e.g., from hwy::BTreeSet). Operates directly on the external state // without copying data, and leaves node deallocation to the external owner. +// +// Iterator invalidation: as with absl::btree_*, any mutation (insert, erase, +// insert_or_assign, operator[], clear, Build, swap, assignment) may invalidate +// ALL outstanding iterators, pointers and references, including end(). Keys +// are stored delta-compressed inside 512-byte leaves, so an insert can shift, +// recompress or split a leaf, and an erase can merge or free one. To continue +// iterating after a mutation, use the iterator returned by insert() or +// erase(iterator); do not reuse iterators obtained before the mutation. template class BTree { public: @@ -1870,7 +1878,8 @@ class BTree { // --------------------------------------------------------------------------- // Constructs a BTreeSet from an array of pre-sorted, unique keys in - // O(N) time. + // O(N) time. Assigning the result to an existing tree invalidates all of + // that tree's iterators, pointers and references. // // Example usage: // std::vector sorted_keys = {10, 20, 30, 40, 50}; @@ -2203,6 +2212,9 @@ class BTree { // --------------------------------------------------------------------------- // Dynamic Mutations (Insertions & Deletions) // --------------------------------------------------------------------------- + // If an insertion or erasure occurs, every outstanding iterator, pointer and + // reference (including end()) may be invalidated. The returned iterator (if + // any) is valid until the next mutation. // Inserts a key into the Set. Returns pair of (iterator, bool_inserted). template > @@ -2302,10 +2314,30 @@ class BTree { return EraseInternal(KeyCodec::ToStorage(key)); } + // Erases the element at `pos`, which must be dereferenceable (not end()). + // Returns an iterator to the element following the erased one, or end(). + // Like absl::btree, this is the only safe way to erase while iterating: + // for (auto it = t.begin(); it != t.end();) { + // if (ShouldErase(*it)) it = t.erase(it); else ++it; + // } + // Cost is one additional root-to-leaf descent to re-locate the successor, + // because erasing may merge or free the leaf that `pos` pointed into. + iterator erase(const_iterator pos) { + HWY_DASSERT(pos.leaf() != nullptr && pos.slot() < pos.leaf()->NumKeys()); + const StorageKeyT key = GetLeafKey(pos.leaf(), pos.slot()); + const size_t erased = EraseInternal(key); + HWY_DASSERT(erased == 1); + (void)erased; + // The key is gone, so lower_bound yields the first element greater than it. + return LowerBoundInternal(key); + } + // --------------------------------------------------------------------------- // Capacity & Iteration // --------------------------------------------------------------------------- + // Removes all elements and frees all nodes. Invalidates all iterators, + // pointers and references. void clear() { DestroySubtree(state_->root_, state_->tree_height_); state_->root_ = nullptr; diff --git a/hwy/contrib/btree/btree_map.h b/hwy/contrib/btree/btree_map.h index 4fd5dd639d..df2ccc9647 100644 --- a/hwy/contrib/btree/btree_map.h +++ b/hwy/contrib/btree/btree_map.h @@ -60,6 +60,8 @@ struct MapDispatch; static std::pair, bool> Insert( \ State* state, KeyT key, StorageValueT value, bool assign_if_exists); \ static size_t Erase(State* state, KeyT key); \ + static std::pair EraseIter(State* state, const Leaf* leaf, \ + size_t slot); \ static void ContainsBatch(const State* state, const KeyT* keys, \ size_t count, bool* out); \ static void FindBatch(const State* state, const KeyT* keys, size_t count, \ @@ -98,6 +100,20 @@ struct ValueStorageTraits { using Type = std::conditional_t; }; +// SIMD-accelerated ordered map from 32/64-bit integer keys to trivially +// copyable 32/64-bit values, with an API modelled on std::map / +// absl::btree_map. Keys are stored delta-compressed in 512-byte leaves; SIMD +// kernels are selected at runtime via dynamic dispatch. +// +// Iterator invalidation: as with absl::btree_map, if an insertion or erasure +// occurs (insert, insert_or_assign, emplace, operator[], erase, clear, Build, +// swap, assignment), ALL outstanding iterators, pointers and references may be +// invalidated, including end() and ValueT& obtained from operator[]/at/ +// FindValue. To continue iterating after a mutation, use the iterator returned +// by insert() or erase(iterator): +// for (auto it = m.begin(); it != m.end();) { +// if (ShouldErase(it->second)) it = m.erase(it); else ++it; +// } template class BTreeMap { public: @@ -146,9 +162,11 @@ class BTreeMap { return *this; } - // Bulk-builds a tree from sorted keys and values. The contiguous values array - // is passed as const void* across dynamic dispatch to avoid strict-aliasing - // issues across different types sharing the same 32-bit or 64-bit size. + // Bulk-builds a tree from strictly ascending keys and their values. The + // contiguous values array is passed as const void* across dynamic dispatch + // to avoid strict-aliasing issues across different types sharing the same + // 32-bit or 64-bit size. Assigning the result to an existing map invalidates + // all of its iterators. static BTreeMap Build(const KeyT* sorted_keys, const ValueT* sorted_values, size_t num_keys, float fill_ratio = 1.0f) { BTreeMap map; @@ -157,6 +175,7 @@ class BTreeMap { return map; } + // Removes all elements. Invalidates all iterators, pointers and references. void clear() { Dispatch::Clear(&state_); } // --------------------------------------------------------------------------- @@ -556,6 +575,8 @@ class BTreeMap { // Inserts key and value. Converts value to StorageValueT via BitCastScalar, // preventing strict aliasing issues while compiling down to a register move. + // Returns (iterator to the element, whether it was inserted). If an + // insertion occurs, all other iterators are invalidated. std::pair insert(KeyT key, const ValueT& value) { StorageValueT s_val = hwy::BitCastScalar(value); auto res = @@ -576,7 +597,15 @@ class BTreeMap { return {iterator(res.first.first, res.first.second, state_.last_leaf_), res.second}; } + // Erases `key` if present. Returns the number of elements erased (0 or 1). + // If an erasure occurs, all iterators are invalidated. size_t erase(KeyT key) { return Dispatch::Erase(&state_, key); } + // Erases the element at `pos` (must not be end()). Returns an iterator to the + // following element, or end(). All other iterators are invalidated. + iterator erase(const_iterator pos) { + auto res = Dispatch::EraseIter(&state_, pos.leaf(), pos.slot()); + return iterator(res.first, res.second, state_.last_leaf_); + } const LeafT* last_leaf() const { return state_.last_leaf_; } LeafT* last_leaf() { return state_.last_leaf_; } diff --git a/hwy/contrib/btree/btree_set.h b/hwy/contrib/btree/btree_set.h index 3683374c14..4786b333c6 100644 --- a/hwy/contrib/btree/btree_set.h +++ b/hwy/contrib/btree/btree_set.h @@ -25,6 +25,18 @@ namespace hwy { +// SIMD-accelerated ordered set of 32/64-bit integer keys with an API modelled +// on std::set / absl::btree_set. Keys are stored delta-compressed in 512-byte +// leaves; SIMD kernels are selected at runtime via dynamic dispatch. +// +// Iterator invalidation: as with absl::btree_set, if an insertion or erasure +// occurs (insert, emplace, erase, clear, Build, swap, assignment), ALL +// outstanding iterators, pointers and references may be invalidated, +// including end(). To continue iterating after a mutation, use the iterator +// returned by insert() or erase(iterator): +// for (auto it = s.begin(); it != s.end();) { +// if (ShouldErase(*it)) it = s.erase(it); else ++it; +// } template class BTreeSet { public: @@ -60,9 +72,12 @@ class BTreeSet { return *this; } + // Bulk-builds a set from `num_keys` strictly ascending keys in O(N). + // Assigning the result to an existing set invalidates all of its iterators. static BTreeSet Build(const KeyT* sorted_keys, size_t num_keys, float fill_ratio = 1.0f); + // Removes all elements. Invalidates all iterators, pointers and references. void clear(); // --------------------------------------------------------------------------- @@ -380,12 +395,19 @@ class BTreeSet { state_.last_leaf_); } + // Inserts `key`. Returns (iterator to the element, whether it was inserted). + // If an insertion occurs, all other iterators are invalidated. std::pair insert(KeyT key); template std::pair emplace(Args&&... args) { return insert(KeyT(std::forward(args)...)); } + // Erases `key` if present. Returns the number of elements erased (0 or 1). + // If an erasure occurs, all iterators are invalidated. size_t erase(KeyT key); + // Erases the element at `pos` (must not be end()). Returns an iterator to the + // following element, or end(). All other iterators are invalidated. + iterator erase(const_iterator pos); const LeafT* last_leaf() const { return state_.last_leaf_; } LeafT* last_leaf() { return state_.last_leaf_; } @@ -436,6 +458,9 @@ class BTreeSet { template <> \ HWY_CONTRIB_DLLEXPORT size_t BTreeSet::erase(KeyT key); \ template <> \ + HWY_CONTRIB_DLLEXPORT BTreeSet::iterator BTreeSet::erase( \ + const_iterator pos); \ + template <> \ HWY_CONTRIB_DLLEXPORT bool BTreeSet::Contains(KeyT key) const; \ template <> \ HWY_CONTRIB_DLLEXPORT void BTreeSet::ContainsBatch( \ diff --git a/hwy/contrib/btree/btree_test_util-inl.h b/hwy/contrib/btree/btree_test_util-inl.h index 1bd949852d..51ec76738b 100644 --- a/hwy/contrib/btree/btree_test_util-inl.h +++ b/hwy/contrib/btree/btree_test_util-inl.h @@ -5,8 +5,10 @@ #include #include +#include #include #include +#include #include #include @@ -38,7 +40,7 @@ struct IsPairLike : std::false_type {}; template struct IsPairLike().first), - decltype(std::declval().second)> > + decltype(std::declval().second)>> : std::true_type {}; template @@ -75,7 +77,7 @@ struct ValueGenerator { }; template -struct ValueGenerator > { +struct ValueGenerator> { uint64_t max_val; explicit ValueGenerator(uint64_t m) : max_val(m) {} @@ -128,12 +130,12 @@ struct KeyExtractor { }; template -struct KeyExtractor > { +struct KeyExtractor> { static const K& Get(const std::pair& p) { return p.first; } }; template -struct KeyExtractor > { +struct KeyExtractor> { static const K& Get(const std::pair& p) { return p.first; } }; @@ -148,14 +150,14 @@ struct ValueComparator { }; template -struct ValueComparator > { +struct ValueComparator> { bool operator()(const std::pair& a, const std::pair& b) const { return a.first < b.first; } }; template -struct ValueComparator > { +struct ValueComparator> { bool operator()(const std::pair& a, const std::pair& b) const { return a.first < b.first; @@ -168,14 +170,14 @@ struct ValueEquality { }; template -struct ValueEquality > { +struct ValueEquality> { bool operator()(const std::pair& a, const std::pair& b) const { return a.first == b.first; } }; template -struct ValueEquality > { +struct ValueEquality> { bool operator()(const std::pair& a, const std::pair& b) const { return a.first == b.first; @@ -902,7 +904,7 @@ void DoTypedefsAndObserversTest() { static_assert(requires { typename TreeT::const_pointer; }); static_assert(requires { typename TreeT::allocator_type; }); static_assert( - std::is_same_v >); + std::is_same_v>); // 2. Runtime comparator observer verification (matching absl::btree tests) TreeT tree; @@ -1114,6 +1116,175 @@ void DoExtremeBoundariesTest() { DoFullContainerTest(vals, /*seed=*/77777); } +// Verifies erase(iterator): the returned iterator must designate the same +// element as the std:: reference's erase(iterator), across leaf merges, leaf +// frees, and first_leaf_/last_leaf_ updates. Runs on a dense (8-bit delta) +// and a sparse (wide delta / raw) key distribution. +template +void DoEraseIteratorTest() { + using value_type = typename TreeT::value_type; + using key_type = typename TreeT::key_type; + using Checker = BTreeChecker; + + // Map iterators yield proxy references (not std::pair), so extract the key + // structurally rather than via KeyExtractor. + const auto key_of = [](const auto& v) -> key_type { + if constexpr (IsPairLike>::value) { + return v.first; + } else { + return v; + } + }; + + const size_t n = AdjustedReps(4000); + const std::vector> datasets = { + GenerateValuesWithSeed(n, n + n / 4, /*seed=*/4242), + GenerateValuesWithSeed(n, n * 100000ULL, /*seed=*/4243), + }; + + for (const auto& values : datasets) { + // 1. Single forward pass, erasing every element whose key hash hits a + // predicate. Compare the returned iterator to std::'s at every step. + { + TreeT tree; + StdRefT ref; + for (const auto& v : values) { + tree.insert(v); + ref.insert(v); + } + Checker::VerifyPhysicalTree(tree); + + auto it = tree.begin(); + auto ref_it = ref.begin(); + size_t erased = 0; + while (it != tree.end()) { + HWY_ASSERT(ref_it != ref.end()); + VerifyEqualElements(it, ref_it); + const uint64_t k = static_cast(key_of(*it)); + if ((k * 0x9E3779B97F4A7C15ULL) >> 62 != 0) { // erase ~75% + it = tree.erase(it); + ref_it = ref.erase(ref_it); + ++erased; + HWY_ASSERT_EQ(tree.size(), ref.size()); + if (it == tree.end()) { + HWY_ASSERT(ref_it == ref.end()); + } else { + HWY_ASSERT(ref_it != ref.end()); + VerifyEqualElements(it, ref_it); + } + if ((erased & 255) == 0) { + Checker::VerifyPhysicalTree(tree); + } + } else { + ++it; + ++ref_it; + } + } + HWY_ASSERT(ref_it == ref.end()); + HWY_ASSERT_EQ(tree.size(), ref.size()); + Checker::VerifyPhysicalTree(tree); + // Remaining elements must match exactly. + auto t2 = tree.begin(); + for (auto r2 = ref.begin(); r2 != ref.end(); ++r2, ++t2) { + HWY_ASSERT(t2 != tree.end()); + VerifyEqualElements(t2, r2); + } + HWY_ASSERT(t2 == tree.end()); + } + + // 2. Erase from the front until empty: returned iterator is the new + // begin(); exercises leaf frees and first_leaf_ maintenance. + { + TreeT tree; + StdRefT ref; + for (const auto& v : values) { + tree.insert(v); + ref.insert(v); + } + size_t steps = 0; + while (!tree.empty()) { + auto it = tree.erase(tree.begin()); + ref.erase(ref.begin()); + HWY_ASSERT(it == tree.begin()); + HWY_ASSERT_EQ(tree.size(), ref.size()); + if (tree.empty()) { + HWY_ASSERT(it == tree.end()); + } else { + VerifyEqualElements(it, ref.begin()); + } + if ((++steps & 511) == 0) { + Checker::VerifyPhysicalTree(tree); + } + } + HWY_ASSERT(tree.begin() == tree.end()); + HWY_ASSERT(tree.state()->root_ == nullptr); + } + + // 3. Erase from the back until empty: returned iterator is always end(); + // exercises last_leaf_ maintenance (and thus --end()). + { + TreeT tree; + StdRefT ref; + for (const auto& v : values) { + tree.insert(v); + ref.insert(v); + } + size_t steps = 0; + while (!tree.empty()) { + auto last = tree.end(); + --last; + auto it = tree.erase(last); + ref.erase(std::prev(ref.end())); + HWY_ASSERT(it == tree.end()); + HWY_ASSERT_EQ(tree.size(), ref.size()); + if (!tree.empty()) { + auto fresh_last = tree.end(); + --fresh_last; + VerifyEqualElements(fresh_last, std::prev(ref.end())); + } + if ((++steps & 511) == 0) { + Checker::VerifyPhysicalTree(tree); + } + } + HWY_ASSERT(tree.state()->root_ == nullptr); + } + + // 4. Erase the last key of every leaf: the successor always lives in the + // next leaf, so the re-descent must cross leaf boundaries correctly. + { + TreeT tree; + StdRefT ref; + for (const auto& v : values) { + tree.insert(v); + ref.insert(v); + } + // Collect last-key-of-leaf positions first (erasing mutates leaves). + std::vector targets; + for (auto it = tree.begin(); it != tree.end(); ++it) { + if (it.slot() + 1 == it.leaf()->NumKeys()) { + targets.push_back(key_of(*it)); + } + } + HWY_ASSERT(targets.size() >= 2); // multi-leaf tree + for (const key_type k : targets) { + auto pos = tree.find(k); + HWY_ASSERT(pos != tree.end()); + auto ref_pos = ref.find(k); + auto it = tree.erase(pos); + auto ref_it = ref.erase(ref_pos); + if (ref_it == ref.end()) { + HWY_ASSERT(it == tree.end()); + } else { + HWY_ASSERT(it != tree.end()); + VerifyEqualElements(it, ref_it); + } + } + HWY_ASSERT_EQ(tree.size(), ref.size()); + Checker::VerifyPhysicalTree(tree); + } + } +} + template void RunFullTestSuite() { DoTypedefsAndObserversTest(); @@ -1121,6 +1292,7 @@ void RunFullTestSuite() { DoBoundarySizeSweep(); DoDiverseBitModesTest(); DoExtremeBoundariesTest(); + DoEraseIteratorTest(); // Multi-level scale across sorted, rsorted, and random orderings (matching // absl::btree_test.cc's BtreeTest()). AdjustedReps ensures ASan/MSan/QEMU diff --git a/hwy/contrib/btree/btreemap_impl-inl.h b/hwy/contrib/btree/btreemap_impl-inl.h index b3c26318ea..912ca4fc06 100644 --- a/hwy/contrib/btree/btreemap_impl-inl.h +++ b/hwy/contrib/btree/btreemap_impl-inl.h @@ -160,6 +160,14 @@ void MapEraseImpl(TreeState* state, BTREE_KEY_T key, size_t* out_erased) { *out_erased = tree.erase(key); } +void MapEraseIterImpl(TreeState* state, const TreeLeaf* leaf, size_t slot, + TreeLeaf** out_leaf, size_t* out_slot) { + TreeEngine tree(state); + auto it = tree.erase(typename TreeEngine::const_iterator(leaf, slot)); + *out_leaf = it.leaf(); + *out_slot = it.slot(); +} + void MapContainsBatchImpl(const TreeState* state, const BTREE_KEY_T* HWY_RESTRICT keys, size_t count, bool* HWY_RESTRICT out) { @@ -219,6 +227,7 @@ HWY_EXPORT(MapFindImpl); HWY_EXPORT(MapFindValueImpl); HWY_EXPORT(MapInsertImpl); HWY_EXPORT(MapEraseImpl); +HWY_EXPORT(MapEraseIterImpl); // Defines the dynamic dispatch entry points for MapDispatch. StorageValueT is strictly uint32_t or uint64_t; user-facing @@ -306,6 +315,17 @@ size_t MapDispatch::Erase(State* state, return erased; } +std::pair::Leaf*, size_t> +MapDispatch::EraseIter(State* state, + const Leaf* leaf, + size_t slot) { + Leaf* out_leaf = nullptr; + size_t out_slot = 0; + HWY_DYNAMIC_DISPATCH(MapEraseIterImpl)(state, leaf, slot, &out_leaf, + &out_slot); + return {out_leaf, out_slot}; +} + void MapDispatch::ContainsBatch( const State* state, const BTREE_KEY_T* keys, size_t count, bool* out) { HWY_DYNAMIC_DISPATCH(MapContainsBatchImpl)(state, keys, count, out); diff --git a/hwy/contrib/btree/btreeset_impl-inl.h b/hwy/contrib/btree/btreeset_impl-inl.h index 6323fe3226..c46919f414 100644 --- a/hwy/contrib/btree/btreeset_impl-inl.h +++ b/hwy/contrib/btree/btreeset_impl-inl.h @@ -135,6 +135,15 @@ void SetEraseImpl(TreeState* state, BTREE_KEY_T key, size_t* out_erased) { *out_erased = tree.erase(key); } +void SetEraseIterImpl(TreeState* state, const typename TreeEngine::Leaf* leaf, + size_t slot, + typename hwy::BTreeSet::iterator* out_it) { + TreeEngine tree(state); + auto it = tree.erase(typename TreeEngine::const_iterator(leaf, slot)); + *out_it = typename hwy::BTreeSet::iterator(it.leaf(), it.slot(), + state->last_leaf_); +} + void SetContainsBatchImpl(const TreeState* state, const BTREE_KEY_T* HWY_RESTRICT keys, size_t count, bool* HWY_RESTRICT out) { @@ -187,6 +196,7 @@ HWY_EXPORT(SetUpperBoundImpl); HWY_EXPORT(SetFindImpl); HWY_EXPORT(SetInsertImpl); HWY_EXPORT(SetEraseImpl); +HWY_EXPORT(SetEraseIterImpl); template <> void BTreeSet::clear() { @@ -268,6 +278,14 @@ size_t BTreeSet::erase(BTREE_KEY_T key) { return erased; } +template <> +BTreeSet::iterator BTreeSet::erase( + const_iterator pos) { + BTreeSet::iterator it; + HWY_DYNAMIC_DISPATCH(SetEraseIterImpl)(&state_, pos.leaf(), pos.slot(), &it); + return it; +} + template <> bool BTreeSet::Contains(BTREE_KEY_T key) const { return HWY_DYNAMIC_DISPATCH(SetContainsImpl)(&state_, key);