From a3cb879e49cca8c361149a33134ae40d5e84a9d6 Mon Sep 17 00:00:00 2001 From: Chungmin Lee Date: Sat, 31 Oct 2026 07:54:51 +0000 Subject: [PATCH 1/2] Make HLL sketch and union exception-safe on allocation failure An allocation failure inside an HLL sketch or union could leak memory, leave a sketch pointing to freed memory, or leave the sketch or union changed or internally inconsistent. Allocate objects through a helper that keeps the allocator's pointer type until construction succeeds and frees storage if the constructor throws. Use it for construction, copies, conversions, promotions and deserialization. Use copy-and-swap for sketch and union assignment, including self-assignment. Roll back LIST, SET and AuxHashMap insertions when growth fails, and update the auxiliary table size only after the larger table is in place. Copy auxiliary maps with an explicit source allocator for both the object and its entries. Preserve table order even when container copy selection would choose an unequal allocator. Move LIST and SET growth and promotion, with their rollback, into non-inlined functions so the ordinary coupon path stays small. Mark the sketch constructor and object helper inline to retain inexpensive LIST construction. In HLL_4, create and fill the auxiliary map before writing AUX_TOKEN. Keep auxiliary updates out of the inlined register path. Check for the last minimum register before changing it. On the ordinary non-auxiliary update path, update the estimator before the non-allocating register writes. Validate and decrement registers in one pass per curMin step, restoring the written registers if a consistency check or allocation throws. Keep replacement auxiliary maps owned until they are installed. During conversion, use the same update path with the iterator's slot directly; the delegating constructor cleans up if conversion throws. Merge sparse source coupons through a local implementation pointer, updating the union's owning pointer after each promotion. Retain the union's allocator for inputs with unequal allocators, including guarded HLL copies and downsampling at the smaller precision. Sweep those paths with per-allocator accounting and check source-pool lifetimes. The sketch documentation now states: "Allocation failures propagate without leaking storage. Updates, copy assignment and reset leave the sketch unchanged if an allocation fails." The union documentation now states: "Allocation failures propagate without leaking storage. If an allocation fails while a LIST or SET sketch is merged into a union in LIST or SET mode, the union stays valid and usable, holding the coupons inserted before the failure. Repeating the merge gives the result of an uninterrupted merge. All other updates, copy assignment and reset leave the union unchanged if an allocation fails." This also fixes a heap buffer overflow when serializing after failed auxiliary growth, and a use-after-free when stream deserialization finds the wrong number of auxiliary entries. Reject overfull compact SET images before replaying their coupons, so replay cannot lose a promoted implementation. Check the SET precision before computing the compact count limit. Add allocation-failure sweeps that check cleanup, unchanged destinations where guaranteed, continued use and merge retries. Compare allocator pointer and raw-pointer results, and check construction-failure cleanup. Check null construction, assignment and comparisons in both operand orders. Exercise selecting allocators on curMin shifts, conversions and failures, recording mismatched releases and balancing each pool. Check that rejected curMin shifts on malformed HLL_4 images leave the sketch unchanged. Preserve the original consistency exceptions where both implementations reject an image. A curMin shift also rejects HLL_4 images that mark a slot as an exception without an auxiliary entry; the previous code accepted them. Fixes #551 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5852c4a7-7589-49ef-bf80-8f2050501a9d --- hll/include/AuxHashMap-internal.hpp | 42 +- hll/include/AuxHashMap.hpp | 1 + hll/include/CouponHashSet-internal.hpp | 51 +- hll/include/CouponHashSet.hpp | 7 +- hll/include/CouponList-internal.hpp | 31 +- hll/include/CouponList.hpp | 8 + hll/include/Hll4Array-internal.hpp | 299 +++-- hll/include/Hll4Array.hpp | 16 +- hll/include/Hll6Array-internal.hpp | 4 +- hll/include/Hll8Array-internal.hpp | 4 +- hll/include/HllSketch-internal.hpp | 9 +- hll/include/HllSketchImplFactory.hpp | 34 +- hll/include/HllUnion-internal.hpp | 30 +- hll/include/HllUtil.hpp | 20 + hll/include/hll.hpp | 38 +- hll/test/AllocationFailureTest.cpp | 1391 ++++++++++++++++++++++++ hll/test/CMakeLists.txt | 1 + 17 files changed, 1747 insertions(+), 239 deletions(-) create mode 100644 hll/test/AllocationFailureTest.cpp diff --git a/hll/include/AuxHashMap-internal.hpp b/hll/include/AuxHashMap-internal.hpp index a89ac0ed..91072f7d 100644 --- a/hll/include/AuxHashMap-internal.hpp +++ b/hll/include/AuxHashMap-internal.hpp @@ -35,14 +35,23 @@ auxCount(0), entries(1ULL << lgAuxArrInts, 0, allocator) {} +template +AuxHashMap::AuxHashMap(const AuxHashMap& that, const A& allocator): +lgConfigK(that.lgConfigK), +lgAuxArrInts(that.lgAuxArrInts), +auxCount(that.auxCount), +entries(that.entries, allocator) +{} + template AuxHashMap* AuxHashMap::newAuxHashMap(uint8_t lgAuxArrInts, uint8_t lgConfigK, const A& allocator) { - return new (ahmAlloc(allocator).allocate(1)) AuxHashMap(lgAuxArrInts, lgConfigK, allocator); + return HllUtil::template newObject>(allocator, lgAuxArrInts, lgConfigK, allocator); } template AuxHashMap* AuxHashMap::newAuxHashMap(const AuxHashMap& that) { - return new (ahmAlloc(that.entries.get_allocator()).allocate(1)) AuxHashMap(that); + const A allocator(that.entries.get_allocator()); + return HllUtil::template newObject>(allocator, that, allocator); } template @@ -60,12 +69,14 @@ AuxHashMap* AuxHashMap::deserialize(const void* bytes, size_t len, const uint32_t configKmask = (1 << lgConfigK) - 1; AuxHashMap* auxHashMap; + std::unique_ptr, std::function*)>> aux_ptr(nullptr, make_deleter()); const uint32_t* auxPtr = static_cast(bytes); if (srcCompact) { if (len < auxCount * sizeof(int)) { throw std::out_of_range("Input array too small to hold AuxHashMap image"); } - auxHashMap = new (ahmAlloc(allocator).allocate(1)) AuxHashMap(lgArrInts, lgConfigK, allocator); + auxHashMap = newAuxHashMap(lgArrInts, lgConfigK, allocator); + aux_ptr.reset(auxHashMap); for (uint32_t i = 0; i < auxCount; ++i) { const uint32_t pair = auxPtr[i]; const uint32_t slotNo = HllUtil::getLow26(pair) & configKmask; @@ -77,7 +88,8 @@ AuxHashMap* AuxHashMap::deserialize(const void* bytes, size_t len, if (len < itemsToRead * sizeof(uint32_t)) { throw std::out_of_range("Input array too small to hold AuxHashMap image"); } - auxHashMap = new (ahmAlloc(allocator).allocate(1)) AuxHashMap(lgArrInts, lgConfigK, allocator); + auxHashMap = newAuxHashMap(lgArrInts, lgConfigK, allocator); + aux_ptr.reset(auxHashMap); for (uint32_t i = 0; i < itemsToRead; ++i) { const uint32_t pair = auxPtr[i]; if (pair == hll_constants::EMPTY) { continue; } @@ -88,11 +100,10 @@ AuxHashMap* AuxHashMap::deserialize(const void* bytes, size_t len, } if (auxHashMap->getAuxCount() != auxCount) { - make_deleter()(auxHashMap); throw std::invalid_argument("Deserialized AuxHashMap has wrong number of entries"); } - return auxHashMap; + return aux_ptr.release(); } template @@ -106,7 +117,7 @@ AuxHashMap* AuxHashMap::deserialize(std::istream& is, uint8_t lgConfigK, lgArrInts = lgAuxArrInts; } - AuxHashMap* auxHashMap = new (ahmAlloc(allocator).allocate(1)) AuxHashMap(lgArrInts, lgConfigK, allocator); + AuxHashMap* auxHashMap = newAuxHashMap(lgArrInts, lgConfigK, allocator); typedef std::unique_ptr, std::function*)>> aux_hash_map_ptr; aux_hash_map_ptr aux_ptr(auxHashMap, auxHashMap->make_deleter()); @@ -131,7 +142,6 @@ AuxHashMap* AuxHashMap::deserialize(std::istream& is, uint8_t lgConfigK, } if (auxHashMap->getAuxCount() != auxCount) { - make_deleter()(auxHashMap); throw std::invalid_argument("Deserialized AuxHashMap has wrong number of entries"); } @@ -149,7 +159,7 @@ std::function*)> AuxHashMap::make_deleter() { template AuxHashMap* AuxHashMap::copy() const { - return new (ahmAlloc(entries.get_allocator()).allocate(1)) AuxHashMap(*this); + return newAuxHashMap(*this); } template @@ -189,7 +199,13 @@ void AuxHashMap::mustAdd(uint32_t slotNo, uint8_t value) { // found empty entry entries[~index] = entry_pair; ++auxCount; - checkGrow(); + try { + checkGrow(); + } catch (...) { + entries[~index] = hll_constants::EMPTY; + --auxCount; + throw; + } } template @@ -224,17 +240,19 @@ void AuxHashMap::checkGrow() { template void AuxHashMap::growAuxSpace() { const int configKmask = (1 << lgConfigK) - 1; - const int newArrLen = 1 << ++lgAuxArrInts; + const uint8_t newLgAuxArrInts = static_cast(lgAuxArrInts + 1); + const int newArrLen = 1 << newLgAuxArrInts; vector_int entries_new(newArrLen, 0, entries.get_allocator()); for (size_t i = 0; i < entries.size(); ++i) { const uint32_t fetched = entries[i]; if (fetched != hll_constants::EMPTY) { // find empty in new array - const int32_t idx = find(entries_new.data(), lgAuxArrInts, lgConfigK, fetched & configKmask); + const int32_t idx = find(entries_new.data(), newLgAuxArrInts, lgConfigK, fetched & configKmask); entries_new[~idx] = fetched; } } entries = std::move(entries_new); + lgAuxArrInts = newLgAuxArrInts; } //Searches the Aux arr hash table for an empty or a matching slotNo depending on the context. diff --git a/hll/include/AuxHashMap.hpp b/hll/include/AuxHashMap.hpp index d4b155b4..9a9b1bce 100644 --- a/hll/include/AuxHashMap.hpp +++ b/hll/include/AuxHashMap.hpp @@ -32,6 +32,7 @@ template class AuxHashMap final { public: AuxHashMap(uint8_t lgAuxArrInts, uint8_t lgConfigK, const A& allocator); + AuxHashMap(const AuxHashMap& that, const A& allocator); static AuxHashMap* newAuxHashMap(uint8_t lgAuxArrInts, uint8_t lgConfigK, const A& allocator); static AuxHashMap* newAuxHashMap(const AuxHashMap& that); diff --git a/hll/include/CouponHashSet-internal.hpp b/hll/include/CouponHashSet-internal.hpp index 2ec4d6a8..d3415db4 100644 --- a/hll/include/CouponHashSet-internal.hpp +++ b/hll/include/CouponHashSet-internal.hpp @@ -83,12 +83,20 @@ CouponHashSet* CouponHashSet::newSet(const void* bytes, size_t len, const if (lgK <= 7) { throw std::invalid_argument("Attempt to deserialize invalid CouponHashSet with lgConfigK <= 7. Found: " + std::to_string(lgK)); - } + } + HllUtil::checkLgK(lgK); uint8_t lgArrInts = data[hll_constants::LG_ARR_BYTE]; const bool compactFlag = ((data[hll_constants::FLAGS_BYTE] & hll_constants::COMPACT_FLAG_MASK) ? true : false); uint32_t couponCount; std::memcpy(&couponCount, data + hll_constants::HASH_SET_COUNT_INT, sizeof(couponCount)); + const uint32_t maxCouponCount = hll_constants::RESIZE_NUMER * (uint32_t(1) << (lgK - 3)) + / hll_constants::RESIZE_DENOM; + if (compactFlag && couponCount > maxCouponCount) { + throw std::invalid_argument("Attempt to deserialize invalid compact CouponHashSet with couponCount > capacity. Found couponCount: " + + std::to_string(couponCount) + + ", capacity: " + std::to_string(maxCouponCount)); + } if (lgArrInts < hll_constants::LG_INIT_SET_SIZE) { lgArrInts = HllUtil<>::computeLgArrInts(SET, couponCount, lgK); } @@ -101,8 +109,8 @@ CouponHashSet* CouponHashSet::newSet(const void* bytes, size_t len, const + ", found: " + std::to_string(len)); } - ChsAlloc chsa(allocator); - CouponHashSet* sketch = new (chsa.allocate(1)) CouponHashSet(lgK, tgtHllType, allocator); + auto sketch = HllUtil::template newObject>(allocator, lgK, tgtHllType, allocator); + std::unique_ptr, std::function*)>> ptr(sketch, sketch->get_deleter()); if (compactFlag) { const uint8_t* curPos = data + hll_constants::HASH_SET_INT_ARR_START; @@ -119,7 +127,7 @@ CouponHashSet* CouponHashSet::newSet(const void* bytes, size_t len, const couponsInArray * sizeof(uint32_t)); } - return sketch; + return ptr.release(); } template @@ -149,16 +157,23 @@ CouponHashSet* CouponHashSet::newSet(std::istream& is, const A& allocator) throw std::invalid_argument("Attempt to deserialize invalid CouponHashSet with lgConfigK <= 7. Found: " + std::to_string(lgK)); } + HllUtil::checkLgK(lgK); uint8_t lgArrInts = listHeader[hll_constants::LG_ARR_BYTE]; const bool compactFlag = ((listHeader[hll_constants::FLAGS_BYTE] & hll_constants::COMPACT_FLAG_MASK) ? true : false); const auto couponCount = read(is); + const uint32_t maxCouponCount = hll_constants::RESIZE_NUMER * (uint32_t(1) << (lgK - 3)) + / hll_constants::RESIZE_DENOM; + if (compactFlag && couponCount > maxCouponCount) { + throw std::invalid_argument("Attempt to deserialize invalid compact CouponHashSet with couponCount > capacity. Found couponCount: " + + std::to_string(couponCount) + + ", capacity: " + std::to_string(maxCouponCount)); + } if (lgArrInts < hll_constants::LG_INIT_SET_SIZE) { lgArrInts = HllUtil<>::computeLgArrInts(SET, couponCount, lgK); } - ChsAlloc chsa(allocator); - CouponHashSet* sketch = new (chsa.allocate(1)) CouponHashSet(lgK, tgtHllType, allocator); + auto sketch = HllUtil::template newObject>(allocator, lgK, tgtHllType, allocator); typedef std::unique_ptr, std::function*)>> coupon_hash_set_ptr; coupon_hash_set_ptr ptr(sketch, sketch->get_deleter()); @@ -183,14 +198,12 @@ CouponHashSet* CouponHashSet::newSet(std::istream& is, const A& allocator) template CouponHashSet* CouponHashSet::copy() const { - ChsAlloc chsa(this->coupons_.get_allocator()); - return new (chsa.allocate(1)) CouponHashSet(*this); + return HllUtil::template newObject>(this->getAllocator(), *this); } template CouponHashSet* CouponHashSet::copyAs(target_hll_type tgtHllType) const { - ChsAlloc chsa(this->coupons_.get_allocator()); - return new (chsa.allocate(1)) CouponHashSet(*this, tgtHllType); + return HllUtil::template newObject>(this->getAllocator(), *this, tgtHllType); } template @@ -202,8 +215,8 @@ HllSketchImpl* CouponHashSet::couponUpdate(uint32_t coupon) { } this->coupons_[~index] = coupon; // found empty ++this->couponCount_; - if (checkGrowOrPromote()) { - return this->promoteHeapListOrSetToHll(*this); + if (static_cast(hll_constants::RESIZE_DENOM * this->couponCount_) > (hll_constants::RESIZE_NUMER * this->coupons_.size())) { + return growOrPromoteWithRollback(static_cast(~index)); } return this; } @@ -219,15 +232,19 @@ uint8_t CouponHashSet::getPreInts() const { } template -bool CouponHashSet::checkGrowOrPromote() { - if (static_cast(hll_constants::RESIZE_DENOM * this->couponCount_) > (hll_constants::RESIZE_NUMER * this->coupons_.size())) { +HllSketchImpl* CouponHashSet::growOrPromoteWithRollback(uint32_t index) { + try { const uint8_t lgCouponArrInts = count_trailing_zeros_in_u32(static_cast(this->coupons_.size())); if (lgCouponArrInts == (this->lgConfigK_ - 3)) { // at max size - return true; // promote to HLL + return this->promoteHeapListOrSetToHll(*this); } - growHashSet(lgCouponArrInts + 1); + growHashSet(static_cast(lgCouponArrInts + 1)); + } catch (...) { + this->coupons_[index] = hll_constants::EMPTY; + --this->couponCount_; + throw; } - return false; + return this; } template diff --git a/hll/include/CouponHashSet.hpp b/hll/include/CouponHashSet.hpp index fd221060..770b46dc 100644 --- a/hll/include/CouponHashSet.hpp +++ b/hll/include/CouponHashSet.hpp @@ -50,7 +50,12 @@ class CouponHashSet : public CouponList { private: using ChsAlloc = typename std::allocator_traits::template rebind_alloc>; - bool checkGrowOrPromote(); +#if defined(_MSC_VER) + __declspec(noinline) +#elif defined(__GNUC__) || defined(__clang__) + __attribute__((noinline)) +#endif + HllSketchImpl* growOrPromoteWithRollback(uint32_t index); void growHashSet(uint8_t tgtLgCoupArrSize); }; diff --git a/hll/include/CouponList-internal.hpp b/hll/include/CouponList-internal.hpp index de48239f..69cad73a 100644 --- a/hll/include/CouponList-internal.hpp +++ b/hll/include/CouponList-internal.hpp @@ -59,14 +59,12 @@ std::function*)> CouponList::get_deleter() const { template CouponList* CouponList::copy() const { - ClAlloc cla(coupons_.get_allocator()); - return new (cla.allocate(1)) CouponList(*this); + return HllUtil::template newObject>(getAllocator(), *this); } template CouponList* CouponList::copyAs(target_hll_type tgtHllType) const { - ClAlloc cla(coupons_.get_allocator()); - return new (cla.allocate(1)) CouponList(*this, tgtHllType); + return HllUtil::template newObject>(getAllocator(), *this, tgtHllType); } template @@ -113,8 +111,7 @@ CouponList* CouponList::newList(const void* bytes, size_t len, const A& al + ", found: " + std::to_string(len)); } - ClAlloc cla(allocator); - CouponList* sketch = new (cla.allocate(1)) CouponList(lgK, tgtHllType, mode, allocator); + auto sketch = HllUtil::template newObject>(allocator, lgK, tgtHllType, mode, allocator); sketch->couponCount_ = couponCount; sketch->putOutOfOrderFlag(oooFlag); // should always be false for LIST @@ -162,8 +159,7 @@ CouponList* CouponList::newList(std::istream& is, const A& allocator) { + ", capacity: " + std::to_string(listCapacity)); } - ClAlloc cla(allocator); - CouponList* sketch = new (cla.allocate(1)) CouponList(lgK, tgtHllType, mode, allocator); + auto sketch = HllUtil::template newObject>(allocator, lgK, tgtHllType, mode, allocator); using coupon_list_ptr = std::unique_ptr, std::function*)>>; coupon_list_ptr ptr(sketch, sketch->get_deleter()); sketch->couponCount_ = couponCount; @@ -287,10 +283,7 @@ HllSketchImpl* CouponList::couponUpdate(uint32_t coupon) { coupons_[i] = coupon; // the actual update ++couponCount_; if (couponCount_ == static_cast(coupons_.size())) { // array full - if (this->lgConfigK_ < 8) { - return promoteHeapListOrSetToHll(*this); - } - return promoteHeapListToSet(*this); + return promoteWithRollback(static_cast(i)); } return this; } @@ -303,6 +296,20 @@ HllSketchImpl* CouponList::couponUpdate(uint32_t coupon) { throw std::runtime_error("Array invalid: no empties and no duplicates"); } +template +HllSketchImpl* CouponList::promoteWithRollback(uint32_t index) { + try { + if (this->lgConfigK_ < 8) { + return promoteHeapListOrSetToHll(*this); + } + return promoteHeapListToSet(*this); + } catch (...) { + coupons_[index] = hll_constants::EMPTY; + --couponCount_; + throw; + } +} + template double CouponList::getCompositeEstimate() const { return getEstimate(); } diff --git a/hll/include/CouponList.hpp b/hll/include/CouponList.hpp index f57b3a06..73a6905a 100644 --- a/hll/include/CouponList.hpp +++ b/hll/include/CouponList.hpp @@ -85,6 +85,14 @@ class CouponList : public HllSketchImpl { vector_int coupons_; friend class HllSketchImplFactory; + + private: +#if defined(_MSC_VER) + __declspec(noinline) +#elif defined(__GNUC__) || defined(__clang__) + __attribute__((noinline)) +#endif + HllSketchImpl* promoteWithRollback(uint32_t index); }; } diff --git a/hll/include/Hll4Array-internal.hpp b/hll/include/Hll4Array-internal.hpp index adfed74a..dcd783e5 100644 --- a/hll/include/Hll4Array-internal.hpp +++ b/hll/include/Hll4Array-internal.hpp @@ -22,6 +22,7 @@ #include "Hll4Array.hpp" +#include #include #include #include @@ -53,15 +54,12 @@ Hll4Array::Hll4Array(const Hll4Array& that) : template Hll4Array::Hll4Array(const HllArray& other) : - HllArray(other.getLgConfigK(), target_hll_type::HLL_4, other.getAllocator()), - auxHashMap_(nullptr) + Hll4Array(other.getLgConfigK(), other.getAllocator()) { - const int numBytes = this->hll4ArrBytes(this->lgConfigK_); - this->hllByteArr_.resize(numBytes, 0); this->oooFlag_ = other.isOutOfOrderFlag(); for (const auto coupon : other) { // all = false, so skip empty values - internalCouponUpdate(coupon); // updates KxQ registers + internalHll4Update(HllUtil::getLow26(coupon), HllUtil::getValue(coupon)); } this->hipAccum_ = other.getHipAccum(); this->rebuild_kxq_curmin_ = false; @@ -88,9 +86,7 @@ std::function*)> Hll4Array::get_deleter() const { template Hll4Array* Hll4Array::copy() const { - using Hll4Alloc = typename std::allocator_traits::template rebind_alloc>; - Hll4Alloc hll4Alloc(this->getAllocator()); - return new (hll4Alloc.allocate(1)) Hll4Array(*this); + return HllUtil::template newObject>(this->getAllocator(), *this); } template @@ -165,169 +161,170 @@ void Hll4Array::putSlot(uint32_t slotNo, uint8_t newValue) { } } -//In C: two-registers.c Line 836 in "hhb_abstract_set_slot_if_new_value_bigger" non-sparse template void Hll4Array::internalHll4Update(uint32_t slotNo, uint8_t newVal) { + const uint8_t rawStoredOldValue = getSlot(slotNo); + const uint8_t lbOnOldValue = rawStoredOldValue + this->curMin_; + if (newVal <= lbOnOldValue) return; + const uint8_t shiftedNewValue = static_cast(newVal - this->curMin_); + if (rawStoredOldValue == hll_constants::AUX_TOKEN || shiftedNewValue >= hll_constants::AUX_TOKEN) { + internalHll4UpdateAux(slotNo, newVal, rawStoredOldValue); + return; + } + if (rawStoredOldValue == 0) { + if (this->numAtCurMin_ == 1) { + shiftToBiggerCurMin(slotNo, newVal); + return; + } + --this->numAtCurMin_; + } + this->hipAndKxQIncrementalUpdate(lbOnOldValue, newVal); + putSlot(slotNo, shiftedNewValue); +} - const uint8_t rawStoredOldValue = getSlot(slotNo); // could be a 0 - // this is provably a LB: - const uint8_t lbOnOldValue = rawStoredOldValue + this->curMin_; // lower bound, could be 0 - - if (newVal > lbOnOldValue) { // 842 - // Note: if an AUX_TOKEN exists, then auxHashMap must already exist - // 846: rawStoredOldValue == AUX_TOKEN - const uint8_t actualOldValue = (rawStoredOldValue < hll_constants::AUX_TOKEN) - ? (lbOnOldValue) : (auxHashMap_->mustFindValueFor(slotNo)); - - if (newVal > actualOldValue) { // 848: actualOldValue could still be 0; newValue > 0 - // we know that the array will change, but we haven't actually updated yet - this->hipAndKxQIncrementalUpdate(actualOldValue, newVal); - - // newVal >= curMin - - const uint8_t shiftedNewValue = newVal - this->curMin_; // 874 - // redundant since we know newVal >= curMin, - // and lgConfigK bounds do not allow overflowing an int - //assert(shiftedNewValue >= 0); +template +void Hll4Array::internalHll4UpdateAux(uint32_t slotNo, uint8_t newVal, uint8_t rawStoredOldValue) { + const uint8_t actualOldValue = rawStoredOldValue < hll_constants::AUX_TOKEN + ? static_cast(rawStoredOldValue + this->curMin_) : auxHashMap_->mustFindValueFor(slotNo); + if (newVal <= actualOldValue) return; + + if (actualOldValue == this->curMin_ && this->numAtCurMin_ == 1) { + shiftToBiggerCurMin(slotNo, newVal); + return; + } - if (rawStoredOldValue == hll_constants::AUX_TOKEN) { // 879 - // Given that we have an AUX_TOKEN, there are 4 cases for how to - // actually modify the data structure + const uint8_t shiftedNewValue = static_cast(newVal - this->curMin_); + if (rawStoredOldValue == hll_constants::AUX_TOKEN) { + auxHashMap_->mustReplace(slotNo, newVal); + } else if (shiftedNewValue >= hll_constants::AUX_TOKEN) { + if (auxHashMap_ == nullptr) { + createAuxHashMap(slotNo, newVal); + } else { + auxHashMap_->mustAdd(slotNo, newVal); + } + putSlot(slotNo, hll_constants::AUX_TOKEN); + } else { + putSlot(slotNo, shiftedNewValue); + } - if (shiftedNewValue >= hll_constants::AUX_TOKEN) { // case 1: 881 - // the byte array already contains aux token - // This is the case where old and new values are both exceptions. - // The 4-bit array already is AUX_TOKEN, only need to update auxHashMap - auxHashMap_->mustReplace(slotNo, newVal); - } - else { // case 2: 885 - // This is the hypothetical case where the old value is an exception and the new one is not, - // which is impossible given that curMin has not changed here and newVal > oldValue - } - } else { // rawStoredOldValue != AUX_TOKEN - if (shiftedNewValue >= hll_constants::AUX_TOKEN) { // case 3: 892 - // This is the case where the old value is not an exception and the new value is. - // The AUX_TOKEN must be stored in the 4-bit array and the new value - // added to the exception table - putSlot(slotNo, hll_constants::AUX_TOKEN); - if (auxHashMap_ == nullptr) { - auxHashMap_ = AuxHashMap::newAuxHashMap(hll_constants::LG_AUX_ARR_INTS[this->lgConfigK_], - this->lgConfigK_, this->getAllocator()); - } - auxHashMap_->mustAdd(slotNo, newVal); - } - else { // case 4: 897 - // This is the case where neither the old value nor the new value is an exception. - // We just overwrite the 4-bit array with the shifted new value. - putSlot(slotNo, shiftedNewValue); - } - } + this->hipAndKxQIncrementalUpdate(actualOldValue, newVal); + if (actualOldValue == this->curMin_) --this->numAtCurMin_; +} - // we just increased a pair value, so it might be time to change curMin - if (actualOldValue == this->curMin_) { // 908 - --(this->numAtCurMin_); - while (this->numAtCurMin_ == 0) { - shiftToBiggerCurMin(); // increases curMin by 1, builds a new aux table - // shifts values in 4-bit table and recounts curMin - } - } - } // end newVal <= actualOldValue - } // end newValue <= lbOnOldValue -> return, no need to update array +template +void Hll4Array::createAuxHashMap(uint32_t slotNo, uint8_t value) { + auto deleter = [this](AuxHashMap* map) { + using Alloc = typename std::allocator_traits::template rebind_alloc>; + Alloc alloc(this->getAllocator()); + map->~AuxHashMap(); + alloc.deallocate(map, 1); + }; + std::unique_ptr, decltype(deleter)> ptr( + AuxHashMap::newAuxHashMap(hll_constants::LG_AUX_ARR_INTS[this->lgConfigK_], + this->lgConfigK_, this->getAllocator()), deleter); + ptr->mustAdd(slotNo, value); + auxHashMap_ = ptr.release(); } -// This scheme only works with two double registers (2 kxq values). -// HipAccum, kxq0 and kxq1 remain untouched. -// This changes curMin, numAtCurMin, hllByteArr and auxMap. -// Entering this routine assumes that all slots have valid values > 0 and <= 15. -// An AuxHashMap must exist if any values in the current hllByteArray are already 15. -// In C: again-two-registers.c Lines 710 "hhb_shift_to_bigger_curmin" template -void Hll4Array::shiftToBiggerCurMin() { - const uint8_t newCurMin = this->curMin_ + 1; +void Hll4Array::shiftToBiggerCurMin(uint32_t slotNo, uint8_t newVal) { const uint32_t configK = 1 << this->lgConfigK_; - const uint32_t configKmask = configK - 1; + const uint8_t oldStoredValue = getSlot(slotNo); + using aux_ptr = std::unique_ptr, std::function*)>>; + aux_ptr pending(nullptr, AuxHashMap::make_deleter()); + const AuxHashMap* current = auxHashMap_; + if (newVal - this->curMin_ >= hll_constants::AUX_TOKEN) { + pending.reset(current == nullptr + ? AuxHashMap::newAuxHashMap(hll_constants::LG_AUX_ARR_INTS[this->lgConfigK_], + this->lgConfigK_, this->getAllocator()) + : current->copy()); + if (oldStoredValue == hll_constants::AUX_TOKEN) pending->mustReplace(slotNo, newVal); + else pending->mustAdd(slotNo, newVal); + current = pending.get(); + } + putSlot(slotNo, std::min(static_cast(newVal - this->curMin_), hll_constants::AUX_TOKEN)); + uint8_t curMin = this->curMin_; uint32_t numAtNewCurMin = 0; - uint32_t numAuxTokens = 0; - - // Walk through the slots of 4-bit array decrementing stored values by one unless it - // equals AUX_TOKEN, where it is left alone but counted to be checked later. - // If oldStoredValue is 0 it is an error. - // If the decremented value is 0, we increment numAtNewCurMin. - // Because getNibble is masked to 4 bits oldStoredValue can never be > 15 or negative - for (uint32_t i = 0; i < configK; i++) { //724 - uint8_t oldStoredValue = getSlot(i); - if (oldStoredValue == 0) { - throw std::runtime_error("Array slots cannot be 0 at this point."); - } - if (oldStoredValue < hll_constants::AUX_TOKEN) { - putSlot(i, --oldStoredValue); - if (oldStoredValue == 0) { numAtNewCurMin++; } - } else { //oldStoredValue == AUX_TOKEN - numAuxTokens++; - if (auxHashMap_ == nullptr) { - throw std::logic_error("auxHashMap cannot be null at this point"); + uint32_t shiftedSlots = 0; + try { + do { + const uint8_t newCurMin = static_cast(curMin + 1); + numAtNewCurMin = 0; + uint32_t numAuxTokens = 0; + for (shiftedSlots = 0; shiftedSlots < configK; ++shiftedSlots) { + uint8_t value = getSlot(shiftedSlots); + if (value == 0) { + throw std::runtime_error("Array slots cannot be 0 at this point."); + } + if (value < hll_constants::AUX_TOKEN) { + putSlot(shiftedSlots, --value); + if (value == 0) ++numAtNewCurMin; + } else { + ++numAuxTokens; + if (current == nullptr) { + throw std::logic_error("auxHashMap cannot be null at this point"); + } + } } - } - } - // If old AuxHashMap exists, walk through it updating some slots and build a new AuxHashMap - // if needed. - AuxHashMap* newAuxMap = nullptr; - if (auxHashMap_ != nullptr) { - uint32_t slotNum; - uint8_t oldActualVal; - uint8_t newShiftedVal; - - for (const auto coupon: *auxHashMap_) { - slotNum = HllUtil::getLow26(coupon) & configKmask; - oldActualVal = HllUtil::getValue(coupon); - if (oldActualVal < newCurMin) { - throw std::logic_error("oldActualVal < newCurMin when incrementing curMin"); + aux_ptr next(nullptr, AuxHashMap::make_deleter()); + if (current != nullptr) { + for (const auto coupon: *current) { + const uint32_t slot = HllUtil::getLow26(coupon) & (configK - 1); + const uint8_t value = HllUtil::getValue(coupon); + if (value < newCurMin) { + throw std::logic_error("oldActualVal < newCurMin when incrementing curMin"); + } + if (getSlot(slot) != hll_constants::AUX_TOKEN) { + throw std::logic_error("getSlot(slotNum) != AUX_TOKEN for item in auxiliary hash map"); + } + const uint8_t shifted = static_cast(value - newCurMin); + if (shifted < hll_constants::AUX_TOKEN) { + if (shifted != 14) { + throw std::logic_error("newShiftedVal != 14 for item in old auxHashMap despite curMin increment"); + } + putSlot(slot, shifted); + --numAuxTokens; + } else { + if (next == nullptr) { + next.reset(AuxHashMap::newAuxHashMap(hll_constants::LG_AUX_ARR_INTS[this->lgConfigK_], + this->lgConfigK_, this->getAllocator())); + } + next->mustAdd(slot, value); + } + } + } else if (numAuxTokens != 0) { + throw std::logic_error("No auxiliary hash map, but numAuxTokens != 0"); } - newShiftedVal = oldActualVal - newCurMin; - - if (getSlot(slotNum) != hll_constants::AUX_TOKEN) { - throw std::logic_error("getSlot(slotNum) != AUX_TOKEN for item in auxiliary hash map"); + const uint32_t newAuxCount = next == nullptr ? 0 : next->getAuxCount(); + // Preserve the missing-map error if another shift is required. + if ((next != nullptr || numAtNewCurMin != 0) && newAuxCount != numAuxTokens) { + throw std::runtime_error("Inconsistent counts: auxCount: " + std::to_string(newAuxCount) + + ", HLL tokesn: " + std::to_string(numAuxTokens)); } - // Array slot != AUX_TOKEN at getSlot(slotNum); - if (newShiftedVal < hll_constants::AUX_TOKEN) { // 756 - if (newShiftedVal != 14) { - throw std::logic_error("newShiftedVal != 14 for item in old auxHashMap despite curMin increment"); - } - // The former exception value isn't one anymore, so it stays out of new AuxHashMap. - // Correct the AUX_TOKEN value in the HLL array to the newShiftedVal (14). - putSlot(slotNum, newShiftedVal); - numAuxTokens--; - } else { //newShiftedVal >= AUX_TOKEN - // the former exception remains an exception, so must be added to the newAuxMap - if (newAuxMap == nullptr) { - newAuxMap = AuxHashMap::newAuxHashMap(hll_constants::LG_AUX_ARR_INTS[this->lgConfigK_], - this->lgConfigK_, this->getAllocator()); - } - newAuxMap->mustAdd(slotNum, oldActualVal); + pending = std::move(next); + current = pending.get(); + curMin = newCurMin; + shiftedSlots = 0; + } while (numAtNewCurMin == 0); + } catch (...) { + // Undo completed shifts and the written prefix; demoted exceptions saturate back to AUX_TOKEN. + for (uint32_t i = 0; i < configK; ++i) { + const uint8_t value = getSlot(i); + if (value < hll_constants::AUX_TOKEN) { + const uint8_t restored = static_cast(value + curMin - this->curMin_ + (i < shiftedSlots)); + putSlot(i, std::min(restored, hll_constants::AUX_TOKEN)); } - } //end scan of oldAuxMap - } //end if (auxHashMap != null) - else { // oldAuxMap == null - if (numAuxTokens != 0) { - throw std::logic_error("No auxiliary hash map, but numAuxTokens != 0"); } + putSlot(slotNo, oldStoredValue); + throw; } - if (newAuxMap != nullptr) { - if (newAuxMap->getAuxCount() != numAuxTokens) { - throw std::runtime_error("Inconsistent counts: auxCount: " + std::to_string(newAuxMap->getAuxCount()) - + ", HLL tokesn: " + std::to_string(numAuxTokens)); - } - } - - if (auxHashMap_ != nullptr) { - AuxHashMap::make_deleter()(auxHashMap_); - } - auxHashMap_ = newAuxMap; - - this->curMin_ = newCurMin; + this->hipAndKxQIncrementalUpdate(this->curMin_, newVal); + if (auxHashMap_ != nullptr) AuxHashMap::make_deleter()(auxHashMap_); + auxHashMap_ = pending.release(); + this->curMin_ = curMin; this->numAtCurMin_ = numAtNewCurMin; } diff --git a/hll/include/Hll4Array.hpp b/hll/include/Hll4Array.hpp index 7edfb051..f81732cd 100644 --- a/hll/include/Hll4Array.hpp +++ b/hll/include/Hll4Array.hpp @@ -56,7 +56,21 @@ class Hll4Array final : public HllArray { private: void internalCouponUpdate(uint32_t coupon); void internalHll4Update(uint32_t slotNo, uint8_t newVal); - void shiftToBiggerCurMin(); + // Keep auxiliary updates off the ordinary register path. +#if defined(_MSC_VER) + __declspec(noinline) +#elif defined(__GNUC__) || defined(__clang__) + __attribute__((noinline)) +#endif + void internalHll4UpdateAux(uint32_t slotNo, uint8_t newVal, uint8_t rawStoredOldValue); + // Keep first-map allocation off the inlined update path. +#if defined(_MSC_VER) + __declspec(noinline) +#elif defined(__GNUC__) || defined(__clang__) + __attribute__((noinline)) +#endif + void createAuxHashMap(uint32_t slotNo, uint8_t value); + void shiftToBiggerCurMin(uint32_t slotNo, uint8_t newVal); AuxHashMap* auxHashMap_; }; diff --git a/hll/include/Hll6Array-internal.hpp b/hll/include/Hll6Array-internal.hpp index 34c72438..bcc09e84 100644 --- a/hll/include/Hll6Array-internal.hpp +++ b/hll/include/Hll6Array-internal.hpp @@ -66,9 +66,7 @@ std::function*)> Hll6Array::get_deleter() const { template Hll6Array* Hll6Array::copy() const { - using Hll6Alloc = typename std::allocator_traits::template rebind_alloc>; - Hll6Alloc hll6Alloc(this->getAllocator()); - return new (hll6Alloc.allocate(1)) Hll6Array(*this); + return HllUtil::template newObject>(this->getAllocator(), *this); } template diff --git a/hll/include/Hll8Array-internal.hpp b/hll/include/Hll8Array-internal.hpp index 8c008d0f..91286dff 100644 --- a/hll/include/Hll8Array-internal.hpp +++ b/hll/include/Hll8Array-internal.hpp @@ -64,9 +64,7 @@ std::function*)> Hll8Array::get_deleter() const { template Hll8Array* Hll8Array::copy() const { - using Hll8Alloc = typename std::allocator_traits::template rebind_alloc>; - Hll8Alloc hll8Alloc(this->getAllocator()); - return new (hll8Alloc.allocate(1)) Hll8Array(*this); + return HllUtil::template newObject>(this->getAllocator(), *this); } template diff --git a/hll/include/HllSketch-internal.hpp b/hll/include/HllSketch-internal.hpp index bbd5d29d..04b7e41d 100644 --- a/hll/include/HllSketch-internal.hpp +++ b/hll/include/HllSketch-internal.hpp @@ -42,13 +42,12 @@ typedef union { } longDoubleUnion; template -hll_sketch_alloc::hll_sketch_alloc(uint8_t lg_config_k, target_hll_type tgt_type, bool start_full_size, const A& allocator) { +inline hll_sketch_alloc::hll_sketch_alloc(uint8_t lg_config_k, target_hll_type tgt_type, bool start_full_size, const A& allocator) { HllUtil::checkLgK(lg_config_k); if (start_full_size) { sketch_impl = HllSketchImplFactory::newHll(lg_config_k, tgt_type, allocator); } else { - typedef typename std::allocator_traits::template rebind_alloc> clAlloc; - sketch_impl = new (clAlloc(allocator).allocate(1)) CouponList(lg_config_k, tgt_type, hll_mode::LIST, allocator); + sketch_impl = HllUtil::template newObject>(allocator, lg_config_k, tgt_type, hll_mode::LIST, allocator); } } @@ -95,8 +94,8 @@ hll_sketch_alloc::hll_sketch_alloc(HllSketchImpl* that) : template hll_sketch_alloc& hll_sketch_alloc::operator=(const hll_sketch_alloc& other) { - sketch_impl->get_deleter()(sketch_impl); - sketch_impl = other.sketch_impl->copy(); + hll_sketch_alloc copy(other); + std::swap(sketch_impl, copy.sketch_impl); return *this; } diff --git a/hll/include/HllSketchImplFactory.hpp b/hll/include/HllSketchImplFactory.hpp index 2142b535..f13047ca 100644 --- a/hll/include/HllSketchImplFactory.hpp +++ b/hll/include/HllSketchImplFactory.hpp @@ -20,6 +20,8 @@ #ifndef _HLLSKETCHIMPLFACTORY_HPP_ #define _HLLSKETCHIMPLFACTORY_HPP_ +#include +#include #include #include "HllUtil.hpp" @@ -54,24 +56,26 @@ class HllSketchImplFactory final { template CouponHashSet* HllSketchImplFactory::promoteListToSet(const CouponList& list) { - using ChsAlloc = typename std::allocator_traits::template rebind_alloc>; - CouponHashSet* chSet = new (ChsAlloc(list.getAllocator()).allocate(1)) CouponHashSet(list.getLgConfigK(), list.getTgtHllType(), list.getAllocator()); + auto chSet = HllUtil::template newObject>(list.getAllocator(), + list.getLgConfigK(), list.getTgtHllType(), list.getAllocator()); + std::unique_ptr, std::function*)>> ptr(chSet, chSet->get_deleter()); for (const auto coupon: list) { chSet->couponUpdate(coupon); } - return chSet; + return ptr.release(); } template HllArray* HllSketchImplFactory::promoteListOrSetToHll(const CouponList& src) { HllArray* tgtHllArr = HllSketchImplFactory::newHll(src.getLgConfigK(), src.getTgtHllType(), src.getAllocator()); + std::unique_ptr, std::function*)>> ptr(tgtHllArr, tgtHllArr->get_deleter()); tgtHllArr->putKxQ0(1 << src.getLgConfigK()); for (const auto coupon: src) { tgtHllArr->couponUpdate(coupon); } tgtHllArr->putHipAccum(src.getEstimate()); tgtHllArr->putOutOfOrderFlag(false); - return tgtHllArr; + return ptr.release(); } template @@ -109,14 +113,11 @@ template HllArray* HllSketchImplFactory::newHll(uint8_t lgConfigK, target_hll_type tgtHllType, const A& allocator) { switch (tgtHllType) { case HLL_8: - using Hll8Alloc = typename std::allocator_traits::template rebind_alloc>; - return new (Hll8Alloc(allocator).allocate(1)) Hll8Array(lgConfigK, allocator); + return HllUtil::template newObject>(allocator, lgConfigK, allocator); case HLL_6: - using Hll6Alloc = typename std::allocator_traits::template rebind_alloc>; - return new (Hll6Alloc(allocator).allocate(1)) Hll6Array(lgConfigK, allocator); + return HllUtil::template newObject>(allocator, lgConfigK, allocator); case HLL_4: - using Hll4Alloc = typename std::allocator_traits::template rebind_alloc>; - return new (Hll4Alloc(allocator).allocate(1)) Hll4Array(lgConfigK, allocator); + return HllUtil::template newObject>(allocator, lgConfigK, allocator); } throw std::logic_error("Invalid target_hll_type"); } @@ -128,8 +129,8 @@ HllSketchImpl* HllSketchImplFactory::reset(HllSketchImpl* impl, bool fu impl->get_deleter()(impl); return hll; } else { - using ClAlloc = typename std::allocator_traits::template rebind_alloc>; - CouponList* cl = new (ClAlloc(impl->getAllocator()).allocate(1)) CouponList(impl->getLgConfigK(), impl->getTgtHllType(), hll_mode::LIST, impl->getAllocator()); + auto cl = HllUtil::template newObject>(impl->getAllocator(), + impl->getLgConfigK(), impl->getTgtHllType(), hll_mode::LIST, impl->getAllocator()); impl->get_deleter()(impl); return cl; } @@ -137,20 +138,17 @@ HllSketchImpl* HllSketchImplFactory::reset(HllSketchImpl* impl, bool fu template Hll4Array* HllSketchImplFactory::convertToHll4(const HllArray& srcHllArr) { - using Hll4Alloc = typename std::allocator_traits::template rebind_alloc>; - return new (Hll4Alloc(srcHllArr.getAllocator()).allocate(1)) Hll4Array(srcHllArr); + return HllUtil::template newObject>(srcHllArr.getAllocator(), srcHllArr); } template Hll6Array* HllSketchImplFactory::convertToHll6(const HllArray& srcHllArr) { - using Hll6Alloc = typename std::allocator_traits::template rebind_alloc>; - return new (Hll6Alloc(srcHllArr.getAllocator()).allocate(1)) Hll6Array(srcHllArr); + return HllUtil::template newObject>(srcHllArr.getAllocator(), srcHllArr); } template Hll8Array* HllSketchImplFactory::convertToHll8(const HllArray& srcHllArr) { - using Hll8Alloc = typename std::allocator_traits::template rebind_alloc>; - return new (Hll8Alloc(srcHllArr.getAllocator()).allocate(1)) Hll8Array(srcHllArr); + return HllUtil::template newObject>(srcHllArr.getAllocator(), srcHllArr); } } diff --git a/hll/include/HllUnion-internal.hpp b/hll/include/HllUnion-internal.hpp index d102b792..e97c9f19 100644 --- a/hll/include/HllUnion-internal.hpp +++ b/hll/include/HllUnion-internal.hpp @@ -37,6 +37,14 @@ hll_union_alloc::hll_union_alloc(uint8_t lg_max_k, const A& allocator): gadget_(lg_max_k, target_hll_type::HLL_8, false, allocator) {} +template +hll_union_alloc& hll_union_alloc::operator=(const hll_union_alloc& other) { + hll_union_alloc copy(other); + std::swap(lg_max_k_, copy.lg_max_k_); + std::swap(gadget_, copy.gadget_); + return *this; +} + template hll_sketch_alloc hll_union_alloc::get_result(target_hll_type target_type) const { if (gadget_.sketch_impl->getCurMode() == hll_mode::HLL) { @@ -221,23 +229,14 @@ HllSketchImpl* hll_union_alloc::copy_or_downsample(const HllSketchImpl* if (src_lg_k <= tgt_lg_k && src->getAllocator() == allocator) { return src->copyAs(HLL_8); } - typedef typename std::allocator_traits::template rebind_alloc> hll8Alloc; const uint8_t lg_k = std::min(src_lg_k, tgt_lg_k); - Hll8Array* tgtHllArr = new (hll8Alloc(allocator).allocate(1)) Hll8Array(lg_k, allocator); + auto tgtHllArr = HllUtil::template newObject>(allocator, lg_k, allocator); + std::unique_ptr, std::function*)>> ptr(tgtHllArr, tgtHllArr->get_deleter()); tgtHllArr->mergeHll(*src); //both of these are required for isomorphism tgtHllArr->putHipAccum(src->getHipAccum()); tgtHllArr->putOutOfOrderFlag(src->isOutOfOrderFlag()); - return tgtHllArr; -} - -template -inline HllSketchImpl* hll_union_alloc::leak_free_coupon_update(HllSketchImpl* impl, uint32_t coupon) { - HllSketchImpl* result = impl->couponUpdate(coupon); - if (result != impl) { - impl->get_deleter()(impl); - } - return result; + return ptr.release(); } template @@ -255,7 +254,12 @@ void hll_union_alloc::union_impl(const hll_sketch_alloc& sketch, uint8_t l } else { const CouponList* src = static_cast*>(src_impl); for (auto coupon: *src) { - dst_impl = leak_free_coupon_update(dst_impl, coupon); //assignment required + HllSketchImpl* result = dst_impl->couponUpdate(coupon); + if (result != dst_impl) { + dst_impl->get_deleter()(dst_impl); + dst_impl = result; + gadget_.sketch_impl = dst_impl; + } } } } else if (!dst_impl->isEmpty()) { // src is HLL diff --git a/hll/include/HllUtil.hpp b/hll/include/HllUtil.hpp index 5a9e0069..6402da6d 100644 --- a/hll/include/HllUtil.hpp +++ b/hll/include/HllUtil.hpp @@ -27,8 +27,11 @@ #include "ceiling_power_of_2.hpp" #include +#include +#include #include #include +#include namespace datasketches { @@ -136,8 +139,25 @@ class HllUtil final { static uint8_t simpleIntLog2(uint32_t n); // n must be power of 2 static uint8_t computeLgArrInts(hll_mode mode, uint32_t count, uint8_t lgConfigK); static double getRelErr(bool upperBound, bool unioned, uint8_t lgConfigK, uint8_t numStdDev); + + template + static T* newObject(const A& allocator, Args&&... args); }; +template +template +inline T* HllUtil::newObject(const A& allocator, Args&&... args) { + using Alloc = typename std::allocator_traits::template rebind_alloc; + Alloc alloc(allocator); + auto ptr = alloc.allocate(1); + try { + return new (ptr) T(std::forward(args)...); + } catch (...) { + alloc.deallocate(ptr, 1); + throw; + } +} + template inline uint32_t HllUtil::coupon(const uint64_t hash[]) { uint32_t addr26 = hash[0] & hll_constants::KEY_MASK_26; diff --git a/hll/include/hll.hpp b/hll/include/hll.hpp index f160fb14..68cd57ec 100644 --- a/hll/include/hll.hpp +++ b/hll/include/hll.hpp @@ -95,6 +95,9 @@ enum target_hll_type { * (up to about 10% of K), this implementation leverages a new class of estimator * algorithms with significantly better accuracy. * + *

Allocation failures propagate without leaking storage. Updates, copy assignment and + * reset leave the sketch unchanged if an allocation fails. + * *

This sketch also offers the capability of operating off-heap. Given a WritableMemory object * created by the user, the sketch will perform all of its updates and internal phase transitions * in that object, which can actually reside either on-heap or off-heap based on how it is @@ -445,6 +448,12 @@ class hll_sketch_alloc final { *

Second, the internal effective value of log-base-2 of k for the union operation can * change dynamically based on the smallest lg_config_k that the union operation has seen. * + *

Allocation failures propagate without leaking storage. If an allocation fails while + * a LIST or SET sketch is merged into a union in LIST or SET mode, the union stays valid + * and usable, holding the coupons inserted before the failure. Repeating the merge gives + * the result of an uninterrupted merge. All other updates, copy assignment and reset + * leave the union unchanged if an allocation fails. + * * author Jon Malkin * author Lee Rhodes * author Kevin Lang @@ -460,6 +469,32 @@ class hll_union_alloc { */ explicit hll_union_alloc(uint8_t lg_max_k, const A& allocator = A()); + /** + * Copy constructor + * @param that union to be copied + */ + hll_union_alloc(const hll_union_alloc& that) = default; + + /** + * Move constructor + * @param that union to be moved + */ + hll_union_alloc(hll_union_alloc&& that) = default; + + /** + * Copy assignment operator + * @param other union to be copied + * @return reference to this union + */ + hll_union_alloc& operator=(const hll_union_alloc& other); + + /** + * Move assignment operator + * @param other union to be moved + * @return reference to this union + */ + hll_union_alloc& operator=(hll_union_alloc&& other) = default; + /** * Returns the current cardinality estimate * @return the cardinality estimate @@ -648,9 +683,6 @@ class hll_union_alloc { bool is_out_of_order_flag() const; bool is_estimation_mode() const; - // calls couponUpdate on sketch, freeing the old sketch upon changes in hll_mode - static HllSketchImpl* leak_free_coupon_update(HllSketchImpl* impl, uint32_t coupon); - uint8_t lg_max_k_; hll_sketch_alloc gadget_; }; diff --git a/hll/test/AllocationFailureTest.cpp b/hll/test/AllocationFailureTest.cpp new file mode 100644 index 00000000..4f9f3b38 --- /dev/null +++ b/hll/test/AllocationFailureTest.cpp @@ -0,0 +1,1391 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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 "hll.hpp" + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace datasketches { +namespace { + +struct allocation_failure { + long fail_at = -1; + long calls = 0; + bool failed = false; + + void arm(long allocation) { + fail_at = allocation; + calls = 0; + failed = false; + } + + void disarm() { fail_at = -1; } +}; + +template +struct failing_allocator: test_allocator { + using value_type = T; + template struct rebind { using other = failing_allocator; }; + + explicit failing_allocator(allocation_failure& failure): test_allocator(0), failure(&failure) {} + template + failing_allocator(const failing_allocator& other): test_allocator(0), failure(other.failure) {} + + T* allocate(size_t n) { + if (failure->fail_at >= 0 && failure->calls++ == failure->fail_at) { + failure->failed = true; + throw std::bad_alloc(); + } + return test_allocator::allocate(n); + } + + allocation_failure* failure; +}; + +template +bool operator==(const failing_allocator&, const failing_allocator&) { return true; } +template +bool operator!=(const failing_allocator&, const failing_allocator&) { return false; } + +using failure_sketch = hll_sketch_alloc>; +using failure_union = hll_union_alloc>; + +template +class allocator_pointer { +public: + using element_type = T; + using value_type = typename std::remove_cv::type; + using difference_type = std::ptrdiff_t; + using reference = typename std::add_lvalue_reference::type; + using pointer = allocator_pointer; + using iterator_category = std::random_access_iterator_tag; + template using rebind = allocator_pointer; + + allocator_pointer() noexcept: ptr_(nullptr) {} + allocator_pointer(std::nullptr_t) noexcept: ptr_(nullptr) {} + allocator_pointer(T* ptr) noexcept: ptr_(ptr) {} + template::value, int>::type = 0> + allocator_pointer(const allocator_pointer& other) noexcept: ptr_(other.get()) {} + + T* get() const noexcept { return ptr_; } + T* operator->() const noexcept { return ptr_; } + reference operator*() const { return *ptr_; } + reference operator[](difference_type n) const { return ptr_[n]; } + explicit operator bool() const noexcept { return ptr_ != nullptr; } + operator typename std::conditional::value, const void*, void*>::type() const noexcept { + return ptr_; + } + allocator_pointer& operator++() { ++ptr_; return *this; } + allocator_pointer operator++(int) { auto copy = *this; ++*this; return copy; } + allocator_pointer& operator--() { --ptr_; return *this; } + allocator_pointer operator--(int) { auto copy = *this; --*this; return copy; } + allocator_pointer& operator+=(difference_type n) { ptr_ += n; return *this; } + allocator_pointer& operator-=(difference_type n) { ptr_ -= n; return *this; } + // Offsets match every integral type exactly. MSVC otherwise ranks its built-in arithmetic, + // reached through the void* conversion, as equally good and reports an ambiguity. + template + typename std::enable_if::value, allocator_pointer>::type operator+(I n) const { return ptr_ + n; } + template + typename std::enable_if::value, allocator_pointer>::type operator-(I n) const { return ptr_ - n; } + difference_type operator-(const allocator_pointer& other) const { return ptr_ - other.ptr_; } + bool operator==(const allocator_pointer& other) const { return ptr_ == other.ptr_; } + bool operator!=(const allocator_pointer& other) const { return ptr_ != other.ptr_; } + bool operator==(std::nullptr_t) const noexcept { return ptr_ == nullptr; } + bool operator!=(std::nullptr_t) const noexcept { return ptr_ != nullptr; } + friend bool operator==(std::nullptr_t, const allocator_pointer& ptr) noexcept { return ptr.ptr_ == nullptr; } + friend bool operator!=(std::nullptr_t, const allocator_pointer& ptr) noexcept { return ptr.ptr_ != nullptr; } + bool operator<(const allocator_pointer& other) const { return ptr_ < other.ptr_; } + bool operator>(const allocator_pointer& other) const { return ptr_ > other.ptr_; } + bool operator<=(const allocator_pointer& other) const { return ptr_ <= other.ptr_; } + bool operator>=(const allocator_pointer& other) const { return ptr_ >= other.ptr_; } + template + friend typename std::enable_if::value, allocator_pointer>::type operator+(I n, allocator_pointer ptr) { + return ptr + n; + } + + template + static typename std::enable_if::value, allocator_pointer>::type pointer_to(U& value) noexcept { + return std::addressof(value); + } + +private: + T* ptr_; +}; + +template +struct pointer_failing_allocator: failing_allocator { + using value_type = T; + using pointer = allocator_pointer; + using const_pointer = allocator_pointer; + template struct rebind { using other = pointer_failing_allocator; }; + + explicit pointer_failing_allocator(allocation_failure& failure): failing_allocator(failure) {} + template + pointer_failing_allocator(const pointer_failing_allocator& other): failing_allocator(*other.failure) {} + + pointer allocate(size_t n) { return failing_allocator::allocate(n); } + void deallocate(pointer ptr, size_t n) { failing_allocator::deallocate(ptr.get(), n); } +}; + +template +bool operator==(const pointer_failing_allocator&, const pointer_failing_allocator&) { return true; } +template +bool operator!=(const pointer_failing_allocator&, const pointer_failing_allocator&) { return false; } + +struct allocator_pool { + allocation_failure failure; + const int id; + long long allocations = 0; + long long bytes = 0; + + explicit allocator_pool(int id): id(id) {} + + void check_no_leaks() const { + REQUIRE(allocations == 0); + REQUIRE(bytes == 0); + } +}; + +template +struct pool_failing_allocator: failing_allocator { + using value_type = T; + template struct rebind { using other = pool_failing_allocator; }; + + explicit pool_failing_allocator(allocator_pool& pool): failing_allocator(pool.failure), pool(&pool) {} + template + pool_failing_allocator(const pool_failing_allocator& other): + failing_allocator(other.pool->failure), pool(other.pool) {} + + T* allocate(size_t n) { + T* result = failing_allocator::allocate(n); + ++pool->allocations; + pool->bytes += static_cast(n * sizeof(T)); + return result; + } + + void deallocate(T* p, size_t n) { + failing_allocator::deallocate(p, n); + --pool->allocations; + pool->bytes -= static_cast(n * sizeof(T)); + } + + allocator_pool* pool; +}; + +template +bool operator==(const pool_failing_allocator& a, const pool_failing_allocator& b) { + return a.pool->id == b.pool->id; +} +template +bool operator!=(const pool_failing_allocator& a, const pool_failing_allocator& b) { return !(a == b); } + +using pool_failure_sketch = hll_sketch_alloc>; +using pool_failure_union = hll_union_alloc>; + +struct selecting_allocator_state { + struct allocation { + unsigned pool; + size_t bytes; + }; + struct pool_balance { + long long allocations = 0; + long long bytes = 0; + }; + + allocation_failure failure; + pool_balance pools[2]; + std::map live; + unsigned mismatches = 0; + + void check_no_leaks() const { + REQUIRE(mismatches == 0); + REQUIRE(live.empty()); + for (const auto& pool: pools) { + REQUIRE(pool.allocations == 0); + REQUIRE(pool.bytes == 0); + } + } +}; + +template +struct selecting_allocator: failing_allocator { + using value_type = T; + template struct rebind { using other = selecting_allocator; }; + + explicit selecting_allocator(selecting_allocator_state& state, unsigned pool = 0): + failing_allocator(state.failure), state(&state), pool(pool) {} + template + selecting_allocator(const selecting_allocator& other): + failing_allocator(other.state->failure), state(other.state), pool(other.pool) {} + + selecting_allocator select_on_container_copy_construction() const { + return selecting_allocator(*state, 1 - pool); + } + + T* allocate(size_t n) { + T* result = failing_allocator::allocate(n); + state->live.emplace(result, selecting_allocator_state::allocation{pool, n * sizeof(T)}); + ++state->pools[pool].allocations; + state->pools[pool].bytes += static_cast(n * sizeof(T)); + return result; + } + + void deallocate(T* p, size_t n) noexcept { + const auto it = state->live.find(p); + if (it == state->live.end()) { + ++state->mismatches; + return; + } + if (it->second.pool != pool || it->second.bytes != n * sizeof(T)) ++state->mismatches; + --state->pools[it->second.pool].allocations; + state->pools[it->second.pool].bytes -= static_cast(it->second.bytes); + state->live.erase(it); + failing_allocator::deallocate(p, n); + } + + selecting_allocator_state* state; + unsigned pool; +}; + +template +bool operator==(const selecting_allocator& a, const selecting_allocator& b) { + return a.state == b.state && a.pool == b.pool; +} +template +bool operator!=(const selecting_allocator& a, const selecting_allocator& b) { return !(a == b); } + +using selecting_sketch = hll_sketch_alloc>; + +struct allocation_balance { + const long long allocations = test_allocator_net_allocations; + const long long bytes = test_allocator_total_bytes; + + void check_no_leaks() const { + REQUIRE(test_allocator_net_allocations == allocations); + REQUIRE(test_allocator_total_bytes == bytes); + } +}; + +template +void check_equal(const hll_sketch_alloc& actual, const hll_sketch_alloc& expected) { + const auto compact = actual.serialize_compact(); + const auto expected_compact = expected.serialize_compact(); + REQUIRE(compact.size() == expected_compact.size()); + REQUIRE(std::equal(compact.begin(), compact.end(), expected_compact.begin())); + const auto updatable = actual.serialize_updatable(); + const auto expected_updatable = expected.serialize_updatable(); + REQUIRE(updatable.size() == expected_updatable.size()); + REQUIRE(std::equal(updatable.begin(), updatable.end(), expected_updatable.begin())); + REQUIRE(actual.get_estimate() == expected.get_estimate()); + REQUIRE(actual.get_composite_estimate() == expected.get_composite_estimate()); +} + +template +void check_equal(const hll_union_alloc& actual, const hll_union_alloc& expected) { + check_equal(actual.get_result(HLL_8), expected.get_result(HLL_8)); + REQUIRE(actual.get_estimate() == expected.get_estimate()); +} + +template +void update_range(Sketch& sketch, uint64_t begin, uint64_t end) { + for (uint64_t value = begin; value < end; ++value) sketch.update(value); +} + +failure_sketch make_sketch(allocation_failure& failure, target_hll_type type, hll_mode mode, uint8_t lg_k = 12) { + failure_sketch sketch(lg_k, type, false, failing_allocator(failure)); + const uint32_t set_limit = 3 * (1U << (lg_k - 3)) / 4; + const uint32_t count = mode == LIST ? 7 : (mode == SET ? std::min(100U, set_limit) : 1000); + update_range(sketch, 0, count); + if (mode == HLL) sketch.update(uint64_t(14441)); // register value 15, including an HLL_4 exception + REQUIRE((sketch.serialize_compact()[hll_constants::MODE_BYTE] & 3) == mode); + return sketch; +} + +hll_sketch::vector_bytes maximum_set_image(uint8_t lg_k) { + hll_sketch sketch(lg_k, HLL_8); + const uint32_t capacity = uint32_t(1) << (lg_k - 3); + const uint32_t limit = hll_constants::RESIZE_NUMER * capacity / hll_constants::RESIZE_DENOM; + std::vector occupied(capacity, false); + uint32_t count = 0; + for (uint64_t value = 0; count < limit; ++value) { + HashState hash; + HllUtil<>::hash(&value, sizeof(value), DEFAULT_SEED, hash); + const uint32_t slot = HllUtil<>::getLow26(HllUtil<>::coupon(hash)) & (capacity - 1); + // Distinct final buckets keep the compact order independent of replay order. + if (occupied[slot]) continue; + occupied[slot] = true; + sketch.update(value); + ++count; + } + auto image = sketch.serialize_compact(); + REQUIRE((image[hll_constants::MODE_BYTE] & 3) == SET); + uint32_t actual_count; + std::memcpy(&actual_count, image.data() + hll_constants::HASH_SET_COUNT_INT, sizeof(actual_count)); + REQUIRE(actual_count == limit); + return image; +} + +template +bool inject_failure(allocation_failure& failure, long allocation, Operation&& operation) { + failure.arm(allocation); + bool threw = false; + try { + operation(); + } catch (const std::bad_alloc&) { + threw = true; + } + failure.disarm(); + REQUIRE(threw == failure.failed); + return threw; +} + +pool_failure_sketch make_pool_sketch(allocator_pool& pool, uint8_t lg_k, target_hll_type type, uint32_t count) { + pool_failure_sketch sketch(lg_k, type, false, pool_failing_allocator(pool)); + update_range(sketch, 0, count); + REQUIRE((sketch.serialize_compact()[hll_constants::MODE_BYTE] & 3) + == (count == 7 ? LIST : count == 1000 ? HLL : SET)); + return sketch; +} + +long check_pool_union_update(uint8_t source_lg_k, target_hll_type type, uint32_t initial_count, + uint32_t source_count, bool move, bool equal_allocators) { + const allocation_balance balance; + for (long allocation = 0;; ++allocation) { + CAPTURE(source_lg_k, type, initial_count, source_count, move, equal_allocators, allocation); + allocator_pool pool(0), input_pool(1), control_pool(2), control_input_pool(3); + auto& source_pool = equal_allocators ? pool : input_pool; + bool threw; + { + pool_failure_union sketch(12, pool_failing_allocator(pool)); + pool_failure_union control(12, pool_failing_allocator(control_pool)); + update_range(sketch, 10000, 10000 + initial_count); + update_range(control, 10000, 10000 + initial_count); + const auto before = sketch; + const bool partial_merge = source_count != 1000 + && (before.get_result(HLL_8).serialize_compact()[hll_constants::MODE_BYTE] & 3) != HLL; + { + auto source = make_pool_sketch(equal_allocators ? control_pool : control_input_pool, + source_lg_k, type, source_count); + if (move) control.update(std::move(source)); + else control.update(source); + } + control_input_pool.check_no_leaks(); + { + auto source = make_pool_sketch(source_pool, source_lg_k, type, source_count); + const auto source_image = source.serialize_updatable(); + input_pool.failure.arm(0); + threw = inject_failure(pool.failure, allocation, [&] { + if (move) sketch.update(std::move(source)); + else sketch.update(source); + }); + input_pool.failure.disarm(); + REQUIRE(input_pool.failure.calls == 0); + REQUIRE_FALSE(input_pool.failure.failed); + REQUIRE(pool.failure.calls == allocation + (threw ? 1 : 0)); + if (threw || !equal_allocators) REQUIRE(source.serialize_updatable() == source_image); + } + // The union must remain usable after the unequal input's entire pool is released. + input_pool.check_no_leaks(); + if (threw) { + if (!partial_merge) { + check_equal(sketch, before); + } else { + auto partial = sketch.get_result(HLL_8); + const auto image = partial.serialize_updatable(); + auto restored = pool_failure_sketch::deserialize(image.data(), image.size(), + pool_failing_allocator(pool)); + check_equal(partial, restored); + auto probe = sketch; + update_range(probe, 30000, 30200); + update_range(partial, 30000, 30200); + check_equal(probe.get_result(HLL_8), partial); + } + { + auto source = make_pool_sketch(source_pool, source_lg_k, type, source_count); + input_pool.failure.arm(0); + if (move) sketch.update(std::move(source)); + else sketch.update(source); + input_pool.failure.disarm(); + REQUIRE(input_pool.failure.calls == 0); + REQUIRE_FALSE(input_pool.failure.failed); + } + input_pool.check_no_leaks(); + } + check_equal(sketch, control); + REQUIRE(sketch.get_lg_config_k() == control.get_lg_config_k()); + update_range(sketch, 20000, 20200); + update_range(control, 20000, 20200); + check_equal(sketch, control); + } + pool.check_no_leaks(); + input_pool.check_no_leaks(); + control_pool.check_no_leaks(); + control_input_pool.check_no_leaks(); + balance.check_no_leaks(); + if (!threw) return allocation; + } +} + +template +void check_updates(Factory&& make, const std::vector& values) { + const allocation_balance balance; + for (long allocation = 0;; ++allocation) { + CAPTURE(allocation); + allocation_failure failure, control_failure; + { + auto sketch = make(failure); + auto control = make(control_failure); + failure.arm(allocation); + unsigned exceptions = 0; + for (uint64_t value: values) { + try { + sketch.update(value); + } catch (const std::bad_alloc&) { + ++exceptions; + failure.disarm(); + REQUIRE(failure.failed); + check_equal(sketch, control); + sketch.update(value); + } + control.update(value); + } + failure.disarm(); + REQUIRE(exceptions == (failure.failed ? 1 : 0)); + check_equal(sketch, control); + update_range(sketch, 20000, 20200); + update_range(control, 20000, 20200); + check_equal(sketch, control); + } + balance.check_no_leaks(); + if (!failure.failed) break; + } +} + +uint64_t value_for_slot(uint32_t slot, uint8_t minimum, bool exact = false) { + for (uint64_t value = 0;; ++value) { + HashState hash; + HllUtil<>::hash(&value, sizeof(value), DEFAULT_SEED, hash); + const uint32_t coupon = HllUtil<>::coupon(hash); + const uint8_t register_value = HllUtil<>::getValue(coupon); + if ((HllUtil<>::getLow26(coupon) & 15) == slot + && (exact ? register_value == minimum : register_value >= minimum)) return value; + } +} + +const std::vector& selecting_shift_values() { + static const auto values = [] { + std::vector result; + for (uint32_t slot = 0; slot < 16; ++slot) { + result.push_back(value_for_slot(slot, slot < 2 ? 18 : 3, true)); + } + return result; + }(); + return values; +} + +const std::vector& exception_values() { + static const std::vector values = { + value_for_slot(0, 18), value_for_slot(1, 18), value_for_slot(2, 18), + value_for_slot(3, 18), value_for_slot(4, 18) + }; + return values; +} + +enum class shift_error { + empty_slots, missing_map, low_aux, orphan_aux, short_aux, missing_aux, + dropped_aux, duplicate_aux, later_missing_map +}; + +void set_image_slot(hll_sketch::vector_bytes& image, uint32_t slot, uint8_t value) { + auto& byte = image[hll_constants::HLL_BYTE_ARR_START + slot / 2]; + byte = static_cast((slot & 1) ? (byte & 15) | (value << 4) : (byte & 240) | value); +} + +hll_sketch::vector_bytes malformed_shift_image(shift_error error) { + hll_sketch sketch(4, HLL_4, true); + const bool no_map = error == shift_error::missing_map || error == shift_error::later_missing_map; + for (uint32_t slot = 1; slot < 16; ++slot) { + if (error == shift_error::empty_slots && slot == 8) continue; + uint8_t value = static_cast(error == shift_error::later_missing_map ? 2 : 1); + if (slot == 1 && !no_map && error != shift_error::empty_slots) value = 18; + sketch.update(value_for_slot(slot, value, true)); + } + auto image = sketch.serialize_updatable(); + const uint32_t count = 1; + std::memcpy(image.data() + hll_constants::CUR_MIN_COUNT_INT, &count, sizeof(count)); + if (no_map) set_image_slot(image, 1, hll_constants::AUX_TOKEN); + if (error == shift_error::orphan_aux || error == shift_error::duplicate_aux) set_image_slot(image, 1, 1); + if (error == shift_error::missing_aux || error == shift_error::dropped_aux) { + set_image_slot(image, 2, hll_constants::AUX_TOKEN); + } + if (error == shift_error::low_aux || error == shift_error::short_aux + || error == shift_error::dropped_aux || error == shift_error::duplicate_aux) { + bool found = false; + for (size_t offset = hll_constants::HLL_BYTE_ARR_START + 8; offset < image.size(); offset += sizeof(uint32_t)) { + uint32_t coupon; + std::memcpy(&coupon, image.data() + offset, sizeof(coupon)); + if (coupon != 0) { + const uint8_t value = static_cast(error == shift_error::low_aux ? 0 : error == shift_error::short_aux ? 2 : 15); + coupon = HllUtil<>::pair(error == shift_error::duplicate_aux ? 0U : 1U, value); + std::memcpy(image.data() + offset, &coupon, sizeof(coupon)); + found = true; + break; + } + } + REQUIRE(found); + } + return image; +} + +void check_rejected_shift(failure_sketch& sketch, shift_error error, uint8_t new_value) { + const auto item = value_for_slot(0, new_value, true); + if (error == shift_error::empty_slots) { + REQUIRE_THROWS_MATCHES(sketch.update(item), std::runtime_error, + Catch::Matchers::Message("Array slots cannot be 0 at this point.")); + } else if (error == shift_error::duplicate_aux) { + REQUIRE_THROWS_MATCHES(sketch.update(item), std::invalid_argument, + Catch::Matchers::Message("Found a slotNo that should not be there: SlotNo: 0, Value: 15")); + } else if (error == shift_error::dropped_aux || (error == shift_error::missing_map && new_value == 15)) { + REQUIRE_THROWS_MATCHES(sketch.update(item), std::runtime_error, + Catch::Matchers::Message("Inconsistent counts: auxCount: 0, HLL tokesn: 1")); + } else if (error == shift_error::missing_map && new_value == 16) { + REQUIRE_THROWS_MATCHES(sketch.update(item), std::runtime_error, + Catch::Matchers::Message("Inconsistent counts: auxCount: 1, HLL tokesn: 2")); + } else if (error == shift_error::missing_aux) { + REQUIRE_THROWS_MATCHES(sketch.update(item), std::runtime_error, + Catch::Matchers::Message("Inconsistent counts: auxCount: 1, HLL tokesn: 2")); + } else { + const char* message = error == shift_error::missing_map || error == shift_error::later_missing_map + ? "auxHashMap cannot be null at this point" + : error == shift_error::low_aux ? "oldActualVal < newCurMin when incrementing curMin" + : error == shift_error::orphan_aux ? "getSlot(slotNum) != AUX_TOKEN for item in auxiliary hash map" + : "newShiftedVal != 14 for item in old auxHashMap despite curMin increment"; + REQUIRE_THROWS_MATCHES(sketch.update(item), std::logic_error, Catch::Matchers::Message(message)); + } +} + +} // namespace + +TEST_CASE("hll allocation failure: construction", "[hll_allocation_failure]") { + const allocation_balance balance; + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const bool full_size = GENERATE(false, true); + for (long allocation = 0;; ++allocation) { + CAPTURE(type, full_size, allocation); + allocation_failure failure; + const bool threw = inject_failure(failure, allocation, [&] { + failure_sketch sketch(12, type, full_size, failing_allocator(failure)); + REQUIRE(sketch.is_empty()); + }); + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll allocation failure: copy and conversion", "[hll_allocation_failure]") { + const allocation_balance balance; + const auto source_type = GENERATE(HLL_4, HLL_6, HLL_8); + const auto mode = GENERATE(LIST, SET, HLL); + const bool convert = GENERATE(false, true); + auto target_type = source_type; + if (convert) { + const auto converted_type = GENERATE(HLL_4, HLL_6, HLL_8); + target_type = converted_type; + } + for (long allocation = 0;; ++allocation) { + CAPTURE(source_type, target_type, mode, convert, allocation); + allocation_failure failure, control_failure; + bool threw; + { + auto source = make_sketch(failure, source_type, mode); + auto control = make_sketch(control_failure, source_type, mode); + failure_sketch expected(control, convert ? target_type : source_type); + const auto copy = [&] { + auto result = convert ? failure_sketch(source, target_type) : failure_sketch(source); + failure.disarm(); + check_equal(result, expected); + update_range(result, 20000, 20200); + update_range(expected, 20000, 20200); + check_equal(result, expected); + }; + threw = inject_failure(failure, allocation, copy); + check_equal(source, control); + if (threw) copy(); + } + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll allocation failure: copy assignment", "[hll_allocation_failure]") { + const allocation_balance balance; + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const auto source_mode = GENERATE(LIST, SET, HLL); + const auto target_mode = GENERATE(LIST, SET, HLL); + for (long allocation = 0;; ++allocation) { + CAPTURE(type, source_mode, target_mode, allocation); + allocation_failure failure, control_failure; + bool threw; + { + auto source = make_sketch(failure, type, source_mode); + auto source_control = make_sketch(control_failure, type, source_mode); + auto sketch = make_sketch(failure, HLL_8, target_mode); + auto control = make_sketch(control_failure, HLL_8, target_mode); + threw = inject_failure(failure, allocation, [&] { sketch = source; }); + if (threw) { + check_equal(sketch, control); + sketch = source; + } + control = source_control; + check_equal(sketch, control); + check_equal(source, source_control); + update_range(sketch, 20000, 20200); + update_range(control, 20000, 20200); + check_equal(sketch, control); + const auto& same = sketch; + sketch = same; + check_equal(sketch, control); + } + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll allocation failure: reset", "[hll_allocation_failure]") { + const allocation_balance balance; + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const auto mode = GENERATE(LIST, SET, HLL); + const bool full_size = GENERATE(false, true); + for (long allocation = 0;; ++allocation) { + CAPTURE(type, mode, full_size, allocation); + allocation_failure failure, control_failure; + bool threw; + { + auto sketch = make_sketch(failure, type, mode); + auto control = make_sketch(control_failure, type, mode); + threw = inject_failure(failure, allocation, [&] { sketch.reset(full_size); }); + if (threw) { + check_equal(sketch, control); + sketch.reset(full_size); + } + control.reset(full_size); + check_equal(sketch, control); + update_range(sketch, 20000, 21000); + update_range(control, 20000, 21000); + check_equal(sketch, control); + } + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll allocation failure: serialization", "[hll_allocation_failure]") { + const allocation_balance balance; + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const auto mode = GENERATE(LIST, SET, HLL); + const bool compact = GENERATE(false, true); + for (long allocation = 0;; ++allocation) { + CAPTURE(type, mode, compact, allocation); + allocation_failure failure, control_failure; + bool threw; + { + auto sketch = make_sketch(failure, type, mode); + auto control = make_sketch(control_failure, type, mode); + const auto serialize = [&] { + const auto bytes = compact ? sketch.serialize_compact() : sketch.serialize_updatable(); + failure.disarm(); + REQUIRE(bytes == (compact ? control.serialize_compact() : control.serialize_updatable())); + }; + threw = inject_failure(failure, allocation, serialize); + check_equal(sketch, control); + if (threw) serialize(); + update_range(sketch, 20000, 20200); + update_range(control, 20000, 20200); + check_equal(sketch, control); + } + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll allocation failure: deserialization", "[hll_allocation_failure]") { + const allocation_balance balance; + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const auto mode = GENERATE(LIST, SET, HLL); + const bool compact = GENERATE(false, true); + const bool stream = GENERATE(false, true); + for (long allocation = 0;; ++allocation) { + CAPTURE(type, mode, compact, stream, allocation); + allocation_failure failure, control_failure; + bool threw; + { + auto control = make_sketch(control_failure, type, mode); + const auto bytes = compact ? control.serialize_compact() : control.serialize_updatable(); + std::istringstream control_input(std::string(bytes.begin(), bytes.end())); + auto expected = stream ? failure_sketch::deserialize(control_input, failing_allocator(control_failure)) + : failure_sketch::deserialize(bytes.data(), bytes.size(), failing_allocator(control_failure)); + const auto deserialize = [&] { + std::istringstream input(std::string(bytes.begin(), bytes.end())); + auto sketch = stream ? failure_sketch::deserialize(input, failing_allocator(failure)) + : failure_sketch::deserialize(bytes.data(), bytes.size(), failing_allocator(failure)); + failure.disarm(); + check_equal(sketch, expected); + update_range(sketch, 20000, 20200); + update_range(expected, 20000, 20200); + check_equal(sketch, expected); + }; + threw = inject_failure(failure, allocation, deserialize); + if (threw) deserialize(); + } + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll compact SET: maximum valid count", "[hll_allocation_failure]") { + const allocation_balance balance; + const bool stream = GENERATE(false, true); + for (uint8_t lg_k = 8; lg_k <= 21; ++lg_k) { + CAPTURE(lg_k, stream); + const auto image = maximum_set_image(lg_k); + allocation_failure failure; + { + std::istringstream input(std::string(image.begin(), image.end())); + auto sketch = stream ? failure_sketch::deserialize(input, failing_allocator(failure)) + : failure_sketch::deserialize(image.data(), image.size(), failing_allocator(failure)); + const auto restored = sketch.serialize_compact(); + REQUIRE(restored.size() == image.size()); + REQUIRE(std::equal(restored.begin(), restored.end(), image.begin())); + } + balance.check_no_leaks(); + } +} + +TEST_CASE("hll compact SET: rejects overfull count before allocation", "[hll_allocation_failure]") { + const allocation_balance balance; + const bool stream = GENERATE(false, true); + const bool recompute_size = GENERATE(false, true); + for (uint8_t lg_k = 8; lg_k <= 21; ++lg_k) { + CAPTURE(lg_k, stream, recompute_size); + auto image = maximum_set_image(lg_k); + uint32_t count; + std::memcpy(&count, image.data() + hll_constants::HASH_SET_COUNT_INT, sizeof(count)); + std::vector coupons(count); + std::memcpy(coupons.data(), image.data() + hll_constants::HASH_SET_INT_ARR_START, count * sizeof(uint32_t)); + uint32_t extra = HllUtil<>::pair(0, 63); + while (std::find(coupons.begin(), coupons.end(), extra) != coupons.end()) ++extra; + const size_t offset = image.size(); + image.resize(offset + sizeof(extra)); + std::memcpy(image.data() + offset, &extra, sizeof(extra)); + ++count; + std::memcpy(image.data() + hll_constants::HASH_SET_COUNT_INT, &count, sizeof(count)); + if (recompute_size) image[hll_constants::LG_ARR_BYTE] = 0; + std::istringstream input(std::string(image.begin(), image.end())); + allocation_failure failure; + failure.arm(0); + if (stream) { + REQUIRE_THROWS_AS(failure_sketch::deserialize(input, failing_allocator(failure)), std::invalid_argument); + } else { + REQUIRE_THROWS_AS(failure_sketch::deserialize(image.data(), image.size(), failing_allocator(failure)), + std::invalid_argument); + } + failure.disarm(); + REQUIRE(failure.calls == 0); + REQUIRE_FALSE(failure.failed); + balance.check_no_leaks(); + } +} + +TEST_CASE("hll compact SET: rejects invalid precision before allocation", "[hll_allocation_failure]") { + const allocation_balance balance; + const bool stream = GENERATE(false, true); + const uint8_t lg_k = GENERATE(uint8_t(22), uint8_t(255)); + auto image = maximum_set_image(8); + image[hll_constants::LG_K_BYTE] = lg_k; + image[hll_constants::LG_ARR_BYTE] = 0; + std::istringstream input(std::string(image.begin(), image.end())); + allocation_failure failure; + failure.arm(0); + if (stream) { + REQUIRE_THROWS_AS(failure_sketch::deserialize(input, failing_allocator(failure)), std::invalid_argument); + } else { + REQUIRE_THROWS_AS(failure_sketch::deserialize(image.data(), image.size(), failing_allocator(failure)), + std::invalid_argument); + } + failure.disarm(); + REQUIRE(failure.calls == 0); + REQUIRE_FALSE(failure.failed); + balance.check_no_leaks(); +} + +TEST_CASE("hll allocation failure: updates across modes", "[hll_allocation_failure]") { + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const uint8_t lg_k = GENERATE(uint8_t(4), uint8_t(7), uint8_t(12)); + const bool full_size = GENERATE(false, true); + CAPTURE(type, lg_k, full_size); + std::vector values; + for (uint64_t value = 0; value < 1000; ++value) values.push_back(value); + values.push_back(14441); + values.push_back(805951); // raises the same exception slot after a failed first auxiliary allocation + check_updates([&](allocation_failure& failure) { + return failure_sketch(lg_k, type, full_size, failing_allocator(failure)); + }, values); +} + +TEST_CASE("hll allocation failure: auxiliary map growth", "[hll_allocation_failure]") { + const auto& values = exception_values(); + check_updates([&](allocation_failure& failure) { + failure_sketch sketch(4, HLL_4, true, failing_allocator(failure)); + for (size_t i = 1; i < 4; ++i) sketch.update(values[i]); + return sketch; + }, {values[4]}); +} + +TEST_CASE("hll allocation failure: auxiliary deserialization cleanup", "[hll_allocation_failure]") { + const allocation_balance balance; + const bool stream = GENERATE(false, true); + const uint32_t entries[4] = {0, 0, 0, 0}; + allocation_failure failure; + const failing_allocator allocator(failure); + if (stream) { + std::istringstream input(std::string(reinterpret_cast(entries), sizeof(entries))); + REQUIRE_THROWS_AS(AuxHashMap>::deserialize(input, 4, 1, 2, false, allocator), + std::invalid_argument); + } else { + REQUIRE_THROWS_AS(AuxHashMap>::deserialize(entries, sizeof(entries), 4, 1, 2, false, allocator), + std::invalid_argument); + } + balance.check_no_leaks(); +} + +TEST_CASE("hll allocation failure: promotions with auxiliary allocation", "[hll_allocation_failure]") { + const bool set = GENERATE(false, true); + const auto& values = exception_values(); + check_updates([&](allocation_failure& failure) { + failure_sketch sketch(set ? 8 : 4, HLL_4, false, failing_allocator(failure)); + for (size_t i = 1; i < values.size(); ++i) sketch.update(values[i]); + update_range(sketch, 0, set ? 20 : 3); + REQUIRE((sketch.serialize_compact()[hll_constants::MODE_BYTE] & 3) == (set ? SET : LIST)); + return sketch; + }, {set ? uint64_t(20) : uint64_t(3)}); +} + +TEST_CASE("hll allocation failure: curMin shifts", "[hll_allocation_failure]") { + const bool new_exception = GENERATE(false, true); + const auto& values = exception_values(); + check_updates([&](allocation_failure& failure) { + failure_sketch sketch(4, HLL_4, true, failing_allocator(failure)); + for (uint32_t slot = 1; slot < 16; ++slot) { + sketch.update(slot < values.size() ? values[slot] : value_for_slot(slot, 3, true)); + } + return sketch; + }, {new_exception ? values[0] : value_for_slot(0, 3, true)}); +} + +TEST_CASE("hll allocation failure: conversion with auxiliary growth", "[hll_allocation_failure]") { + const allocation_balance balance; + const auto type = GENERATE(HLL_6, HLL_8); + const bool shift = GENERATE(false, true); + const auto& values = exception_values(); + for (long allocation = 0;; ++allocation) { + CAPTURE(type, allocation); + allocation_failure failure, control_failure; + bool threw; + { + failure_sketch source(4, type, true, failing_allocator(failure)); + failure_sketch control(4, type, true, failing_allocator(control_failure)); + for (size_t i = 1; i < values.size(); ++i) { + source.update(values[i]); + control.update(values[i]); + } + if (shift) { + for (uint32_t slot = 0; slot < 16; ++slot) { + if (slot > 0 && slot < values.size()) continue; + const auto item = value_for_slot(slot, 3, true); + source.update(item); + control.update(item); + } + } + failure_sketch expected(control, HLL_4); + const auto convert = [&] { + failure_sketch sketch(source, HLL_4); + failure.disarm(); + check_equal(sketch, expected); + update_range(sketch, 20000, 20200); + update_range(expected, 20000, 20200); + check_equal(sketch, expected); + }; + threw = inject_failure(failure, allocation, convert); + check_equal(source, control); + if (threw) convert(); + } + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll selecting allocator: curMin shift", "[hll_allocation_failure][hll_selecting_allocator]") { + const allocation_balance balance; + selecting_allocator_state state; + const auto& values = selecting_shift_values(); + { + selecting_sketch sketch(4, HLL_4, true, selecting_allocator(state)); + hll_sketch control(4, HLL_4, true); + for (uint32_t slot = 1; slot < 16; ++slot) { + sketch.update(values[slot]); + control.update(values[slot]); + } + REQUIRE(sketch.serialize_updatable()[hll_constants::HLL_CUR_MIN_BYTE] == 0); + sketch.update(values[0]); + control.update(values[0]); + check_equal(sketch, control); + REQUIRE(sketch.serialize_updatable()[hll_constants::HLL_CUR_MIN_BYTE] == 3); + } + state.check_no_leaks(); + balance.check_no_leaks(); +} + +TEST_CASE("hll selecting allocator: conversion", "[hll_allocation_failure][hll_selecting_allocator]") { + const allocation_balance balance; + const auto type = GENERATE(HLL_6, HLL_8); + selecting_allocator_state state; + const auto& values = selecting_shift_values(); + const auto first = value_for_slot(0, 3, true); + static const auto last = value_for_slot(15, 18, true); + { + selecting_sketch source(4, type, true, selecting_allocator(state)); + hll_sketch control(4, type, true); + for (uint32_t slot = 0; slot < 16; ++slot) { + const auto item = slot == 0 ? first : slot == 15 ? last : values[slot]; + source.update(item); + control.update(item); + } + const selecting_sketch converted(source, HLL_4); + const hll_sketch expected(control, HLL_4); + check_equal(converted, expected); + check_equal(source, control); + } + state.check_no_leaks(); + balance.check_no_leaks(); +} + +TEST_CASE("hll selecting allocator: failed curMin shift", "[hll_allocation_failure][hll_selecting_allocator]") { + const allocation_balance balance; + const auto& values = selecting_shift_values(); + for (long allocation = 0;; ++allocation) { + CAPTURE(allocation); + selecting_allocator_state state; + bool threw; + { + selecting_sketch sketch(4, HLL_4, true, selecting_allocator(state)); + hll_sketch control(4, HLL_4, true); + for (uint32_t slot = 1; slot < 16; ++slot) { + sketch.update(values[slot]); + control.update(values[slot]); + } + const auto before = sketch.serialize_updatable(); + threw = inject_failure(state.failure, allocation, [&] { sketch.update(values[0]); }); + REQUIRE(state.failure.calls == allocation + (threw ? 1 : 0)); + if (threw) { + REQUIRE(sketch.serialize_updatable() == before); + check_equal(sketch, control); + sketch.update(values[0]); + } + control.update(values[0]); + check_equal(sketch, control); + } + state.check_no_leaks(); + balance.check_no_leaks(); + if (!threw) { + REQUIRE(allocation > 2); + break; + } + } +} + +TEST_CASE("hll allocation failure: union updates", "[hll_allocation_failure]") { + const allocation_balance balance; + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const auto mode = GENERATE(LIST, SET, HLL); + const uint8_t lg_k = GENERATE(uint8_t(8), uint8_t(12), uint8_t(13)); + const uint32_t initial_count = GENERATE(0U, 7U, 24U, 100U, 1000U); + const bool move = GENERATE(false, true); + for (long allocation = 0;; ++allocation) { + CAPTURE(type, mode, lg_k, initial_count, move, allocation); + allocation_failure failure, control_failure; + bool threw; + { + failure_union sketch(12, failing_allocator(failure)); + failure_union control(12, failing_allocator(control_failure)); + update_range(sketch, 10000, 10000 + initial_count); + update_range(control, 10000, 10000 + initial_count); + auto source = make_sketch(failure, type, mode, lg_k); + auto source_control = make_sketch(control_failure, type, mode, lg_k); + const bool partial_merge = + (sketch.get_result(HLL_8).serialize_compact()[hll_constants::MODE_BYTE] & 3) != HLL && mode != HLL; + const auto update = [&] { + if (move) sketch.update(std::move(source)); + else sketch.update(source); + }; + threw = inject_failure(failure, allocation, update); + if (threw) { + if (!partial_merge) check_equal(sketch, control); + check_equal(source, source_control); + update(); + } + if (move) control.update(std::move(source_control)); + else control.update(source_control); + check_equal(sketch, control); + update_range(sketch, 20000, 20200); + update_range(control, 20000, 20200); + check_equal(sketch, control); + } + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll allocation failure: union reset and result", "[hll_allocation_failure]") { + const allocation_balance balance; + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const auto mode = GENERATE(LIST, SET, HLL); + const bool reset = GENERATE(false, true); + for (long allocation = 0;; ++allocation) { + CAPTURE(type, mode, reset, allocation); + allocation_failure failure, control_failure; + bool threw; + { + failure_union sketch(12, failing_allocator(failure)); + failure_union control(12, failing_allocator(control_failure)); + auto source = make_sketch(failure, type, mode); + auto source_control = make_sketch(control_failure, type, mode); + sketch.update(source); + control.update(source_control); + check_equal(sketch, control); + const auto operation = [&] { + if (reset) sketch.reset(); + else { + const auto result = sketch.get_result(type); + failure.disarm(); + check_equal(result, control.get_result(type)); + } + }; + threw = inject_failure(failure, allocation, operation); + if (threw) { + check_equal(sketch, control); + operation(); + } + if (reset) control.reset(); + check_equal(sketch, control); + update_range(sketch, 20000, 20200); + update_range(control, 20000, 20200); + check_equal(sketch, control); + } + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll allocation failure: union copy assignment", "[hll_allocation_failure]") { + const allocation_balance balance; + const auto mode = GENERATE(LIST, SET, HLL); + for (long allocation = 0;; ++allocation) { + CAPTURE(mode, allocation); + allocation_failure failure, control_failure; + bool threw; + { + failure_union source(8, failing_allocator(failure)); + failure_union source_control(8, failing_allocator(control_failure)); + auto input = make_sketch(failure, HLL_8, mode, 8); + auto control_input = make_sketch(control_failure, HLL_8, mode, 8); + source.update(input); + source_control.update(control_input); + failure_union sketch(12, failing_allocator(failure)); + failure_union control(12, failing_allocator(control_failure)); + update_range(sketch, 10000, 10007); + update_range(control, 10000, 10007); + threw = inject_failure(failure, allocation, [&] { sketch = source; }); + if (threw) { + check_equal(sketch, control); + auto probe = sketch; + auto expected = control; + auto dense = make_sketch(control_failure, HLL_8, HLL); + probe.update(dense); + expected.update(dense); + check_equal(probe, expected); + sketch = source; + } + control = source_control; + check_equal(sketch, control); + update_range(sketch, 20000, 20200); + update_range(control, 20000, 20200); + check_equal(sketch, control); + const auto& same = sketch; + sketch = same; + check_equal(sketch, control); + } + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll allocation failure: sparse union merge retry", "[hll_allocation_failure]") { + const allocation_balance balance; + const uint32_t initial_count = GENERATE(6U, 23U, 95U, 375U); + const uint32_t source_count = GENERATE(7U, 300U); + const bool move = GENERATE(false, true); + for (long allocation = 0;; ++allocation) { + CAPTURE(initial_count, source_count, move, allocation); + allocation_failure failure, control_failure; + bool threw; + { + failure_union sketch(12, failing_allocator(failure)); + failure_union control(12, failing_allocator(control_failure)); + update_range(sketch, 10000, 10000 + initial_count); + update_range(control, 10000, 10000 + initial_count); + failure_sketch source(12, HLL_4, false, failing_allocator(failure)); + update_range(source, 0, source_count); + REQUIRE((source.serialize_compact()[hll_constants::MODE_BYTE] & 3) == (source_count == 7 ? LIST : SET)); + REQUIRE((sketch.get_result(HLL_8).serialize_compact()[hll_constants::MODE_BYTE] & 3) != HLL); + control.update(source); + const auto merge = [&] { + if (move) sketch.update(std::move(source)); + else sketch.update(source); + }; + threw = inject_failure(failure, allocation, merge); + if (threw) { + auto partial = sketch.get_result(HLL_8); + const auto image = partial.serialize_updatable(); + auto restored = failure_sketch::deserialize(image.data(), image.size(), failing_allocator(failure)); + check_equal(partial, restored); + auto probe = sketch; + update_range(probe, 30000, 30200); + update_range(partial, 30000, 30200); + check_equal(probe.get_result(HLL_8), partial); + merge(); + } + check_equal(sketch, control); + update_range(sketch, 20000, 20200); + update_range(control, 20000, 20200); + check_equal(sketch, control); + } + balance.check_no_leaks(); + if (!threw) break; + } +} + +TEST_CASE("hll allocation failure: union value updates", "[hll_allocation_failure]") { + const uint8_t lg_k = GENERATE(uint8_t(4), uint8_t(7), uint8_t(12)); + std::vector values; + for (uint64_t value = 0; value < 1000; ++value) values.push_back(value); + check_updates([&](allocation_failure& failure) { + return failure_union(lg_k, failing_allocator(failure)); + }, values); +} + +TEST_CASE("hll union allocators: LIST into empty union", "[hll_allocation_failure][hll_union_allocator]") { + const uint8_t lg_k = GENERATE(uint8_t(8), uint8_t(12), uint8_t(13)); + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const bool move = GENERATE(false, true); + const bool equal = GENERATE(false, true); + const long allocations = check_pool_union_update(lg_k, type, 0, 7, move, equal); + if (!equal || lg_k != 12 || (move && type == HLL_8)) REQUIRE(allocations == 0); + else REQUIRE(allocations == 2); +} + +TEST_CASE("hll union allocators: SET into empty union", "[hll_allocation_failure][hll_union_allocator]") { + const uint8_t lg_k = GENERATE(uint8_t(8), uint8_t(12), uint8_t(13)); + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const bool move = GENERATE(false, true); + const bool equal = GENERATE(false, true); + const uint32_t count = lg_k == 8 ? 24U : lg_k == 12 ? 300U : 700U; + const long allocations = check_pool_union_update(lg_k, type, 0, count, move, equal); + if (equal && lg_k == 12 && move && type == HLL_8) REQUIRE(allocations == 0); + else REQUIRE(allocations >= 2); +} + +TEST_CASE("hll union allocators: sparse merge promotions", "[hll_allocation_failure][hll_union_allocator]") { + const uint32_t initial_count = GENERATE(6U, 23U, 95U, 383U); + const uint32_t source_count = GENERATE(7U, 300U, 700U); + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const bool move = GENERATE(false, true); + const bool equal = GENERATE(false, true); + const uint8_t lg_k = source_count == 700 ? uint8_t(13) : uint8_t(12); + REQUIRE(check_pool_union_update(lg_k, type, initial_count, source_count, move, equal) > 0); +} + +TEST_CASE("hll union allocators: HLL into empty union", "[hll_allocation_failure][hll_union_allocator]") { + const uint8_t lg_k = GENERATE(uint8_t(8), uint8_t(12)); + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const bool move = GENERATE(false, true); + const bool equal = GENERATE(false, true); + const long allocations = check_pool_union_update(lg_k, type, 0, 1000, move, equal); + REQUIRE(allocations == (equal && move && type == HLL_8 ? 0 : 2)); +} + +TEST_CASE("hll union allocators: HLL source downsampling", "[hll_allocation_failure][hll_union_allocator]") { + const uint32_t initial_count = GENERATE(0U, 7U, 100U); + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const bool move = GENERATE(false, true); + const bool equal = GENERATE(false, true); + REQUIRE(check_pool_union_update(13, type, initial_count, 1000, move, equal) == 2); +} + +TEST_CASE("hll union allocators: HLL into sparse union", "[hll_allocation_failure][hll_union_allocator]") { + const uint32_t initial_count = GENERATE(7U, 100U); + const uint8_t lg_k = GENERATE(uint8_t(8), uint8_t(12)); + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const bool move = GENERATE(false, true); + const bool equal = GENERATE(false, true); + REQUIRE(check_pool_union_update(lg_k, type, initial_count, 1000, move, equal) == 2); +} + +TEST_CASE("hll union allocators: gadget downsampling", "[hll_allocation_failure][hll_union_allocator]") { + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const bool move = GENERATE(false, true); + const bool equal = GENERATE(false, true); + REQUIRE(check_pool_union_update(8, type, 1000, 1000, move, equal) == 2); +} + +TEST_CASE("hll union allocators: sparse input into dense union", "[hll_allocation_failure][hll_union_allocator]") { + const uint32_t source_count = GENERATE(7U, 300U, 700U); + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const bool move = GENERATE(false, true); + const bool equal = GENERATE(false, true); + const uint8_t lg_k = source_count == 700 ? uint8_t(13) : uint8_t(12); + REQUIRE(check_pool_union_update(lg_k, type, 1000, source_count, move, equal) == 0); +} + +TEST_CASE("hll union allocators: dense merge without downsampling", "[hll_allocation_failure][hll_union_allocator]") { + const uint8_t lg_k = GENERATE(uint8_t(12), uint8_t(13)); + const auto type = GENERATE(HLL_4, HLL_6, HLL_8); + const bool move = GENERATE(false, true); + const bool equal = GENERATE(false, true); + REQUIRE(check_pool_union_update(lg_k, type, 1000, 1000, move, equal) == 0); +} + +TEST_CASE("hll malformed shift: preserves rejected image", "[hll_malformed]") { + const allocation_balance balance; + const auto error = GENERATE(shift_error::empty_slots, shift_error::missing_map, shift_error::low_aux, + shift_error::orphan_aux, shift_error::short_aux, shift_error::missing_aux, + shift_error::dropped_aux, shift_error::duplicate_aux, shift_error::later_missing_map); + uint8_t new_value = static_cast( + error == shift_error::duplicate_aux || error == shift_error::later_missing_map ? 15 : 1); + if (error == shift_error::empty_slots) { + const auto value = GENERATE(1, 3, 15, 18); + new_value = static_cast(value); + } else if (error == shift_error::missing_map) { + const auto value = GENERATE(1, 15, 16); + new_value = static_cast(value); + } + const auto image = malformed_shift_image(error); + allocation_failure failure; + { + auto sketch = failure_sketch::deserialize(image.data(), image.size(), failing_allocator(failure)); + const auto before = sketch.serialize_updatable(); + const auto compact = sketch.serialize_compact(); + const double estimate = sketch.get_estimate(); + check_rejected_shift(sketch, error, new_value); + REQUIRE(sketch.serialize_updatable() == before); + REQUIRE(sketch.serialize_compact() == compact); + REQUIRE(sketch.get_estimate() == estimate); + } + balance.check_no_leaks(); +} + +TEST_CASE("hll allocator pointer: nullable operations", "[hll_allocation_failure][hll_allocator_pointer]") { + using pointer = allocator_pointer; + static_assert(std::is_same() == nullptr), bool>::value, "pointer == null"); + static_assert(std::is_same()), bool>::value, "null == pointer"); + static_assert(std::is_same() != nullptr), bool>::value, "pointer != null"); + static_assert(std::is_same()), bool>::value, "null != pointer"); + pointer value_initialized{}; + REQUIRE(value_initialized == nullptr); + pointer p(nullptr); + REQUIRE(p == nullptr); + REQUIRE(nullptr == p); + REQUIRE_FALSE(p != nullptr); + REQUIRE_FALSE(nullptr != p); + uint8_t value = 0; + REQUIRE(std::pointer_traits::pointer_to(value).get() == &value); + p = pointer(&value); + REQUIRE_FALSE(p == nullptr); + REQUIRE_FALSE(nullptr == p); + REQUIRE(p != nullptr); + REQUIRE(nullptr != p); + REQUIRE((p ? true : false)); + p = nullptr; + REQUIRE_FALSE((p ? true : false)); + REQUIRE(p == nullptr); +} + +TEST_CASE("hll allocator pointer: operations match raw pointers", "[hll_allocation_failure][hll_allocator_pointer]") { + const allocation_balance balance; + allocation_failure failure; + using allocator = pointer_failing_allocator; + static_assert(!std::is_convertible::value, "allocator pointer must not convert to T*"); + using sketch = hll_sketch_alloc; + { + sketch source(12, HLL_4, false, allocator(failure)); + hll_sketch expected_source(12, HLL_4); + update_range(source, 0, 1000); + update_range(expected_source, 0, 1000); + source.update(uint64_t(14441)); + expected_source.update(uint64_t(14441)); + const auto source_image = source.serialize_compact(); + uint32_t aux_count; + std::memcpy(&aux_count, source_image.data() + hll_constants::AUX_COUNT_INT, sizeof(aux_count)); + REQUIRE(aux_count > 0); + const sketch copy(source); + const hll_sketch expected_copy(expected_source); + hll_union_alloc result(12, allocator(failure)); + hll_union expected_result(12); + result.update(copy); + expected_result.update(expected_copy); + const auto image = result.get_result(HLL_4).serialize_compact(); + const auto expected_image = expected_result.get_result(HLL_4).serialize_compact(); + REQUIRE(result.get_estimate() == expected_result.get_estimate()); + REQUIRE(image.size() == expected_image.size()); + REQUIRE(std::equal(image.begin(), image.end(), expected_image.begin())); + } + balance.check_no_leaks(); +} + +TEST_CASE("hll allocator pointer: construction failure releases storage", "[hll_allocation_failure][hll_allocator_pointer]") { + const allocation_balance balance; + allocation_failure failure; + REQUIRE(inject_failure(failure, 1, [&] { + hll_sketch_alloc> sketch( + 12, HLL_4, false, pointer_failing_allocator(failure)); + })); + REQUIRE(failure.calls == 2); + balance.check_no_leaks(); +} + +} // namespace datasketches diff --git a/hll/test/CMakeLists.txt b/hll/test/CMakeLists.txt index f63d1a50..e0d39065 100644 --- a/hll/test/CMakeLists.txt +++ b/hll/test/CMakeLists.txt @@ -37,6 +37,7 @@ add_test( target_sources(hll_test PRIVATE + AllocationFailureTest.cpp AuxHashMapTest.cpp CouponHashSetTest.cpp CouponListTest.cpp From 563bbef091beb235bf7f1ab5bae349e965140f53 Mon Sep 17 00:00:00 2001 From: Chungmin Lee Date: Thu, 5 Nov 2026 00:22:37 +0000 Subject: [PATCH 2/2] Trim unreachable HLL_4 branches and redundant failure sweeps ## Summary Remove two unreachable HLL_4 branches and two duplicate allocation-failure test cases. Explain the minimum-shift entry condition and map lifetime. ## Prompting Intent Reduce unnecessary changes while preserving the allocation-failure fix, its guarantees and distinct allocator coverage. ## Linked Sources - https://github.com/apache/datasketches-cpp/pull/565 - https://github.com/apache/datasketches-cpp/issues/551 ## Rationale The auxiliary update caller excludes ordinary values, and the shift scan already rejects exception tokens without a map. Existing sketch-update sweeps cover union scalar forwarding; failed-mutation checks retain their serialization assertions. Keep the outlined first-map helper and the multi-shift transaction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b7960e30-8d3c-4554-82c5-881b981ab3f4 --- hll/include/Hll4Array-internal.hpp | 9 +++---- hll/test/AllocationFailureTest.cpp | 38 ------------------------------ 2 files changed, 3 insertions(+), 44 deletions(-) diff --git a/hll/include/Hll4Array-internal.hpp b/hll/include/Hll4Array-internal.hpp index dcd783e5..fb66a687 100644 --- a/hll/include/Hll4Array-internal.hpp +++ b/hll/include/Hll4Array-internal.hpp @@ -193,18 +193,15 @@ void Hll4Array::internalHll4UpdateAux(uint32_t slotNo, uint8_t newVal, uint8_ return; } - const uint8_t shiftedNewValue = static_cast(newVal - this->curMin_); if (rawStoredOldValue == hll_constants::AUX_TOKEN) { auxHashMap_->mustReplace(slotNo, newVal); - } else if (shiftedNewValue >= hll_constants::AUX_TOKEN) { + } else { if (auxHashMap_ == nullptr) { createAuxHashMap(slotNo, newVal); } else { auxHashMap_->mustAdd(slotNo, newVal); } putSlot(slotNo, hll_constants::AUX_TOKEN); - } else { - putSlot(slotNo, shiftedNewValue); } this->hipAndKxQIncrementalUpdate(actualOldValue, newVal); @@ -228,6 +225,8 @@ void Hll4Array::createAuxHashMap(uint32_t slotNo, uint8_t value) { template void Hll4Array::shiftToBiggerCurMin(uint32_t slotNo, uint8_t newVal) { + // This update removes the last register at curMin. + // Keep the original auxiliary map until all minimum shifts succeed. const uint32_t configK = 1 << this->lgConfigK_; const uint8_t oldStoredValue = getSlot(slotNo); using aux_ptr = std::unique_ptr, std::function*)>>; @@ -294,8 +293,6 @@ void Hll4Array::shiftToBiggerCurMin(uint32_t slotNo, uint8_t newVal) { next->mustAdd(slot, value); } } - } else if (numAuxTokens != 0) { - throw std::logic_error("No auxiliary hash map, but numAuxTokens != 0"); } const uint32_t newAuxCount = next == nullptr ? 0 : next->getAuxCount(); // Preserve the missing-map error if another shift is required. diff --git a/hll/test/AllocationFailureTest.cpp b/hll/test/AllocationFailureTest.cpp index 4f9f3b38..9ba2e7f3 100644 --- a/hll/test/AllocationFailureTest.cpp +++ b/hll/test/AllocationFailureTest.cpp @@ -705,35 +705,6 @@ TEST_CASE("hll allocation failure: reset", "[hll_allocation_failure]") { } } -TEST_CASE("hll allocation failure: serialization", "[hll_allocation_failure]") { - const allocation_balance balance; - const auto type = GENERATE(HLL_4, HLL_6, HLL_8); - const auto mode = GENERATE(LIST, SET, HLL); - const bool compact = GENERATE(false, true); - for (long allocation = 0;; ++allocation) { - CAPTURE(type, mode, compact, allocation); - allocation_failure failure, control_failure; - bool threw; - { - auto sketch = make_sketch(failure, type, mode); - auto control = make_sketch(control_failure, type, mode); - const auto serialize = [&] { - const auto bytes = compact ? sketch.serialize_compact() : sketch.serialize_updatable(); - failure.disarm(); - REQUIRE(bytes == (compact ? control.serialize_compact() : control.serialize_updatable())); - }; - threw = inject_failure(failure, allocation, serialize); - check_equal(sketch, control); - if (threw) serialize(); - update_range(sketch, 20000, 20200); - update_range(control, 20000, 20200); - check_equal(sketch, control); - } - balance.check_no_leaks(); - if (!threw) break; - } -} - TEST_CASE("hll allocation failure: deserialization", "[hll_allocation_failure]") { const allocation_balance balance; const auto type = GENERATE(HLL_4, HLL_6, HLL_8); @@ -1200,15 +1171,6 @@ TEST_CASE("hll allocation failure: sparse union merge retry", "[hll_allocation_f } } -TEST_CASE("hll allocation failure: union value updates", "[hll_allocation_failure]") { - const uint8_t lg_k = GENERATE(uint8_t(4), uint8_t(7), uint8_t(12)); - std::vector values; - for (uint64_t value = 0; value < 1000; ++value) values.push_back(value); - check_updates([&](allocation_failure& failure) { - return failure_union(lg_k, failing_allocator(failure)); - }, values); -} - TEST_CASE("hll union allocators: LIST into empty union", "[hll_allocation_failure][hll_union_allocator]") { const uint8_t lg_k = GENERATE(uint8_t(8), uint8_t(12), uint8_t(13)); const auto type = GENERATE(HLL_4, HLL_6, HLL_8);