From 34f4d7f4ad388d8264225c2aefe048ca9a42cfa2 Mon Sep 17 00:00:00 2001 From: Kim Barrett Date: Wed, 29 Mar 2023 17:18:16 +0000 Subject: [PATCH] 8304759: Add BitMap iterators Reviewed-by: stefank, aboldtch, tschatzl --- src/hotspot/share/utilities/bitMap.cpp | 6 + src/hotspot/share/utilities/bitMap.hpp | 170 ++++++++++++++++++ src/hotspot/share/utilities/bitMap.inline.hpp | 155 ++++++++++++++++ .../gtest/utilities/test_bitMap_iterate.cpp | 50 ++++++ 4 files changed, 381 insertions(+) diff --git a/src/hotspot/share/utilities/bitMap.cpp b/src/hotspot/share/utilities/bitMap.cpp index ba2ef680e6e..c9eb0065797 100644 --- a/src/hotspot/share/utilities/bitMap.cpp +++ b/src/hotspot/share/utilities/bitMap.cpp @@ -654,6 +654,12 @@ void BitMap::write_to(bm_word_t* buffer, size_t buffer_size_in_bytes) const { memcpy(buffer, _map, size_in_bytes()); } +#ifdef ASSERT +void BitMap::IteratorImpl::assert_not_empty() const { + assert(!is_empty(), "empty iterator"); +} +#endif + #ifndef PRODUCT void BitMap::print_on(outputStream* st) const { diff --git a/src/hotspot/share/utilities/bitMap.hpp b/src/hotspot/share/utilities/bitMap.hpp index b0c293d29dd..db821f2174e 100644 --- a/src/hotspot/share/utilities/bitMap.hpp +++ b/src/hotspot/share/utilities/bitMap.hpp @@ -116,6 +116,8 @@ class BitMap { template struct IterateInvoker; + struct IteratorImpl; + // Threshold for performing small range operation, even when large range // operation was requested. Measured in words. static const size_t small_range_words = 32; @@ -279,6 +281,8 @@ class BitMap { // greater than (less than for reverse iteration) the current index will // affect which further indices the operation will be applied to. // + // See also the Iterator and ReverseIterator classes. + // // precondition: beg and end form a valid range for the bitmap. template bool iterate(Function function, idx_t beg, idx_t end) const; @@ -312,6 +316,11 @@ class BitMap { return reverse_iterate(cl, 0, size()); } + class Iterator; + class ReverseIterator; + class RBFIterator; + class ReverseRBFIterator; + // Return the index of the first set (or clear) bit in the range [beg, end), // or end if none found. // precondition: beg and end form a valid range for the bitmap. @@ -385,6 +394,167 @@ class BitMap { #endif }; +// Implementation support for bitmap iteration. While it could be used to +// support bi-directional iteration, it is only intended to be used for +// uni-directional iteration. The directionality is determined by the using +// class. +struct BitMap::IteratorImpl { + const BitMap* _map; + idx_t _cur_beg; + idx_t _cur_end; + + void assert_not_empty() const NOT_DEBUG_RETURN; + + // Constructs an empty iterator. + IteratorImpl(); + + // Constructs an iterator for map, over the range [beg, end). + // May be constructed for one of forward or reverse iteration. + // precondition: beg and end form a valid range for map. + // precondition: either beg == end or + // (1) if for forward iteration, then beg must designate a set bit, + // (2) if for reverse iteration, then end-1 must designate a set bit. + IteratorImpl(const BitMap* map, idx_t beg, idx_t end); + + // Returns true if the remaining iteration range is empty. + bool is_empty() const; + + // Returns the index of the first set bit in the remaining iteration range. + // precondition: !is_empty() + // precondition: constructed for forward iteration. + idx_t first() const; + + // Returns the index of the last set bit in the remaining iteration range. + // precondition: !is_empty() + // precondition: constructed for reverse iteration. + idx_t last() const; + + // Updates first() to the position of the first set bit in the range + // [first() + 1, last()]. The iterator instead becomes empty if there + // aren't any set bits in that range. + // precondition: !is_empty() + // precondition: constructed for forward iteration. + void step_first(); + + // Updates last() to the position of the last set bit in the range + // [first(), last()). The iterator instead becomes empty if there aren't + // any set bits in that range. + // precondition: !is_empty() + // precondition: constructed for reverse iteration. + void step_last(); +}; + +// Provides iteration over the indices of the set bits in a range of a bitmap, +// in increasing order. This is an alternative to the iterate() function. +class BitMap::Iterator { + IteratorImpl _impl; + +public: + // Constructs an empty iterator. + Iterator(); + + // Constructs an iterator for map, over the range [0, map.size()). + explicit Iterator(const BitMap& map); + + // Constructs an iterator for map, over the range [beg, end). + // If there are no set bits in that range, the resulting iterator is empty. + // Otherwise, index() is initially the position of the first set bit in + // that range. + // precondition: beg and end form a valid range for map. + Iterator(const BitMap& map, idx_t beg, idx_t end); + + // Returns true if the remaining iteration range is empty. + bool is_empty() const; + + // Returns the index of the first set bit in the remaining iteration range. + // precondition: !is_empty() + idx_t index() const; + + // Updates index() to the position of the first set bit in the range + // [index(), end), where end was the corresponding constructor argument. + // The iterator instead becomes empty if there aren't any set bits in + // that range. + // precondition: !is_empty() + void step(); + + // Range-based for loop support. + RBFIterator begin() const; + RBFIterator end() const; +}; + +// Provides iteration over the indices of the set bits in a range of a bitmap, +// in decreasing order. This is an alternative to the reverse_iterate() function. +class BitMap::ReverseIterator { + IteratorImpl _impl; + + static idx_t initial_end(const BitMap& map, idx_t beg, idx_t end); + +public: + // Constructs an empty iterator. + ReverseIterator(); + + // Constructs a reverse iterator for map, over the range [0, map.size()). + explicit ReverseIterator(const BitMap& map); + + // Constructs a reverse iterator for map, over the range [beg, end). + // If there are no set bits in that range, the resulting iterator is empty. + // Otherwise, index() is initially the position of the last set bit in + // that range. + // precondition: beg and end form a valid range for map. + ReverseIterator(const BitMap& map, idx_t beg, idx_t end); + + // Returns true if the remaining iteration range is empty. + bool is_empty() const; + + // Returns the index of the last set bit in the remaining iteration range. + // precondition: !is_empty() + idx_t index() const; + + // Updates index() to the position of the last set bit in the range + // [beg, index()), where beg was the corresponding constructor argument. + // The iterator instead becomes empty if there aren't any set bits in + // that range. + // precondition: !is_empty() + void step(); + + // Range-based for loop support. + ReverseRBFIterator begin() const; + ReverseRBFIterator end() const; +}; + +// Provides range-based for loop iteration support. This class is not +// intended for direct use by an application. It provides the functionality +// required by a range-based for loop with an Iterator as the range. +class BitMap::RBFIterator { + friend class Iterator; + + IteratorImpl _impl; + + RBFIterator(const BitMap* map, idx_t beg, idx_t end); + +public: + bool operator!=(const RBFIterator& i) const; + idx_t operator*() const; + RBFIterator& operator++(); +}; + +// Provides range-based for loop reverse iteration support. This class is +// not intended for direct use by an application. It provides the +// functionality required by a range-based for loop with a ReverseIterator +// as the range. +class BitMap::ReverseRBFIterator { + friend class ReverseIterator; + + IteratorImpl _impl; + + ReverseRBFIterator(const BitMap* map, idx_t beg, idx_t end); + +public: + bool operator!=(const ReverseRBFIterator& i) const; + idx_t operator*() const; + ReverseRBFIterator& operator++(); +}; + // CRTP: BitmapWithAllocator exposes the following Allocator interfaces upward to GrowableBitMap. // // bm_word_t* allocate(idx_t size_in_words) const; diff --git a/src/hotspot/share/utilities/bitMap.inline.hpp b/src/hotspot/share/utilities/bitMap.inline.hpp index 200feb5f7b8..d03b25d0cc4 100644 --- a/src/hotspot/share/utilities/bitMap.inline.hpp +++ b/src/hotspot/share/utilities/bitMap.inline.hpp @@ -370,6 +370,161 @@ inline bool BitMap::reverse_iterate(BitMapClosureType* cl, idx_t beg, idx_t end) return reverse_iterate(function, beg, end); } +/// BitMap::IteratorImpl + +inline BitMap::IteratorImpl::IteratorImpl() + : _map(nullptr), _cur_beg(0), _cur_end(0) +{} + +inline BitMap::IteratorImpl::IteratorImpl(const BitMap* map, idx_t beg, idx_t end) + : _map(map), _cur_beg(beg), _cur_end(end) +{} + +inline bool BitMap::IteratorImpl::is_empty() const { + return _cur_beg == _cur_end; +} + +inline BitMap::idx_t BitMap::IteratorImpl::first() const { + assert_not_empty(); + return _cur_beg; +} + +inline BitMap::idx_t BitMap::IteratorImpl::last() const { + assert_not_empty(); + return _cur_end - 1; +} + +inline void BitMap::IteratorImpl::step_first() { + assert_not_empty(); + _cur_beg = _map->find_first_set_bit(_cur_beg + 1, _cur_end); +} + +inline void BitMap::IteratorImpl::step_last() { + assert_not_empty(); + idx_t lastpos = last(); + idx_t pos = _map->find_last_set_bit(_cur_beg, lastpos); + _cur_end = (pos < lastpos) ? (pos + 1) : _cur_beg; +} + +/// BitMap::Iterator + +inline BitMap::Iterator::Iterator() : _impl() {} + +inline BitMap::Iterator::Iterator(const BitMap& map) + : Iterator(map, 0, map.size()) +{} + +inline BitMap::Iterator::Iterator(const BitMap& map, idx_t beg, idx_t end) + : _impl(&map, map.find_first_set_bit(beg, end), end) +{} + +inline bool BitMap::Iterator::is_empty() const { + return _impl.is_empty(); +} + +inline BitMap::idx_t BitMap::Iterator::index() const { + return _impl.first(); +} + +inline void BitMap::Iterator::step() { + _impl.step_first(); +} + +inline BitMap::RBFIterator BitMap::Iterator::begin() const { + return RBFIterator(_impl._map, _impl._cur_beg, _impl._cur_end); +} + +inline BitMap::RBFIterator BitMap::Iterator::end() const { + return RBFIterator(_impl._map, _impl._cur_end, _impl._cur_end); +} + +/// BitMap::ReverseIterator + +inline BitMap::idx_t BitMap::ReverseIterator::initial_end(const BitMap& map, + idx_t beg, + idx_t end) { + idx_t pos = map.find_last_set_bit(beg, end); + return (pos < end) ? (pos + 1) : beg; +} + +inline BitMap::ReverseIterator::ReverseIterator() : _impl() {} + +inline BitMap::ReverseIterator::ReverseIterator(const BitMap& map) + : ReverseIterator(map, 0, map.size()) +{} + +inline BitMap::ReverseIterator::ReverseIterator(const BitMap& map, + idx_t beg, + idx_t end) + : _impl(&map, beg, initial_end(map, beg, end)) +{} + +inline bool BitMap::ReverseIterator::is_empty() const { + return _impl.is_empty(); +} + +inline BitMap::idx_t BitMap::ReverseIterator::index() const { + return _impl.last(); +} + +inline void BitMap::ReverseIterator::step() { + _impl.step_last(); +} + +inline BitMap::ReverseRBFIterator BitMap::ReverseIterator::begin() const { + return ReverseRBFIterator(_impl._map, _impl._cur_beg, _impl._cur_end); +} + +inline BitMap::ReverseRBFIterator BitMap::ReverseIterator::end() const { + return ReverseRBFIterator(_impl._map, _impl._cur_beg, _impl._cur_beg); +} + +/// BitMap::RBFIterator + +inline BitMap::RBFIterator::RBFIterator(const BitMap* map, idx_t beg, idx_t end) + : _impl(map, beg, end) +{} + +inline bool BitMap::RBFIterator::operator!=(const RBFIterator& i) const { + // Shouldn't be comparing RBF iterators from different contexts. + assert(_impl._map == i._impl._map, "mismatched range-based for iterators"); + assert(_impl._cur_end == i._impl._cur_end, "mismatched range-based for iterators"); + return _impl._cur_beg != i._impl._cur_beg; +} + +inline BitMap::idx_t BitMap::RBFIterator::operator*() const { + return _impl.first(); +} + +inline BitMap::RBFIterator& BitMap::RBFIterator::operator++() { + _impl.step_first(); + return *this; +} + +/// BitMap::ReverseRBFIterator + +inline BitMap::ReverseRBFIterator::ReverseRBFIterator(const BitMap* map, + idx_t beg, + idx_t end) + : _impl(map, beg, end) +{} + +inline bool BitMap::ReverseRBFIterator::operator!=(const ReverseRBFIterator& i) const { + // Shouldn't be comparing RBF iterators from different contexts. + assert(_impl._map == i._impl._map, "mismatched range-based for iterators"); + assert(_impl._cur_beg == i._impl._cur_beg, "mismatched range-based for iterators"); + return _impl._cur_end != i._impl._cur_end; +} + +inline BitMap::idx_t BitMap::ReverseRBFIterator::operator*() const { + return _impl.last(); +} + +inline BitMap::ReverseRBFIterator& BitMap::ReverseRBFIterator::operator++() { + _impl.step_last(); + return *this; +} + // Returns a bit mask for a range of bits [beg, end) within a single word. Each // bit in the mask is 0 if the bit is in the range, 1 if not in the range. The // returned mask can be used directly to clear the range, or inverted to set the diff --git a/test/hotspot/gtest/utilities/test_bitMap_iterate.cpp b/test/hotspot/gtest/utilities/test_bitMap_iterate.cpp index ec2528073a0..8abf6fd382e 100644 --- a/test/hotspot/gtest/utilities/test_bitMap_iterate.cpp +++ b/test/hotspot/gtest/utilities/test_bitMap_iterate.cpp @@ -175,6 +175,50 @@ static void test_reverse_iterate_non_closure(const BitMap& map, ASSERT_EQ(closure._data._positions_index, 0u); } +static void test_iterator(const BitMap& map, + const idx_t* positions, + size_t positions_size) { + SCOPED_TRACE("iterate with Iterator"); + size_t positions_index = 0; + for (BitMap::Iterator it{map}; !it.is_empty(); it.step()) { + test_iterate_step(map, it.index(), positions, positions_index++, positions_size); + } + ASSERT_EQ(positions_index, positions_size); +} + +static void test_reverse_iterator(const BitMap& map, + const idx_t* positions, + size_t positions_size) { + SCOPED_TRACE("reverse iterate with Iterator"); + size_t positions_index = positions_size; + for (BitMap::ReverseIterator it{map}; !it.is_empty(); it.step()) { + test_iterate_step(map, it.index(), positions, --positions_index, positions_size); + } + ASSERT_EQ(positions_index, 0u); +} + +static void test_for_loop_iterator(const BitMap& map, + const idx_t* positions, + size_t positions_size) { + SCOPED_TRACE("iterate with range-based for loop"); + size_t positions_index = 0; + for (idx_t index : BitMap::Iterator(map)) { + test_iterate_step(map, index, positions, positions_index++, positions_size); + } + ASSERT_EQ(positions_index, positions_size); +} + +static void test_for_loop_reverse_iterator(const BitMap& map, + const idx_t* positions, + size_t positions_size) { + SCOPED_TRACE("reverse iterate with range-based for loop"); + size_t positions_index = positions_size; + for (idx_t index : BitMap::ReverseIterator(map)) { + test_iterate_step(map, index, positions, --positions_index, positions_size); + } + ASSERT_EQ(positions_index, 0u); +} + static void fill_iterate_map(BitMap& map, const idx_t* positions, size_t positions_size) { @@ -196,6 +240,12 @@ static void test_iterate(BitMap& map, test_reverse_iterate_lambda(map, positions, positions_size); test_reverse_iterate_closure(map, positions, positions_size); test_reverse_iterate_non_closure(map, positions, positions_size); + + test_iterator(map, positions, positions_size); + test_reverse_iterator(map, positions, positions_size); + + test_for_loop_iterator(map, positions, positions_size); + test_for_loop_reverse_iterator(map, positions, positions_size); } TEST(BitMap, iterate_empty) {