From 334b8b8aa2604964f599913e2748ed9f40c6c0cb Mon Sep 17 00:00:00 2001 From: AntiO2 Date: Sun, 11 Oct 2026 16:35:01 +0800 Subject: [PATCH 1/4] [Issue #1411] Serialize concurrent visibility deletion-chain publication --- cpp/pixels-retina/include/TileVisibility.h | 4 +++ cpp/pixels-retina/lib/TileVisibility.cpp | 5 ++++ cpp/pixels-retina/test/TileVisibilityTest.cpp | 26 +++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/cpp/pixels-retina/include/TileVisibility.h b/cpp/pixels-retina/include/TileVisibility.h index fae7665de..b3f168345 100644 --- a/cpp/pixels-retina/include/TileVisibility.h +++ b/cpp/pixels-retina/include/TileVisibility.h @@ -29,6 +29,7 @@ #endif #include "RetinaBase.h" #include +#include #include #include #include @@ -133,6 +134,9 @@ class TileVisibility : public pixels::RetinaBase> { // EXCLUSIVE: recovery replay without readers; historical deletes fold in place. void deleteTileRecordExclusive(uint16_t rowId, uint64_t ts); + // Publish the chain head, tail, and used slots as one tile-local operation. + // Readers must not observe a reserved slot before its item is initialized. + mutable std::mutex chainMutex; std::atomic*> currentVersion; std::atomic tail; std::atomic tailUsed; diff --git a/cpp/pixels-retina/lib/TileVisibility.cpp b/cpp/pixels-retina/lib/TileVisibility.cpp index f6eb63139..775238653 100644 --- a/cpp/pixels-retina/lib/TileVisibility.cpp +++ b/cpp/pixels-retina/lib/TileVisibility.cpp @@ -77,6 +77,7 @@ TileVisibility::~TileVisibility() { template void TileVisibility::deleteTileRecord(uint16_t rowId, uint64_t ts, ReplayMode replayMode) { + std::lock_guard lock(chainMutex); switch (replayMode) { case ReplayMode::NORMAL: appendDeleteChain(rowId, ts); @@ -264,6 +265,7 @@ inline void process_bitmap_block_128(const DeleteIndexBlock *blk, template void TileVisibility::getTileVisibilityBitmap(uint64_t ts, uint64_t* outBitmap) const { + std::lock_guard lock(chainMutex); // Enter epoch protection EpochGuard guard; // Load current version under epoch protection @@ -333,6 +335,7 @@ void TileVisibility::getTileVisibilityBitmap(uint64_t ts, uint64_t* ou template void TileVisibility::collectTileGarbage(uint64_t ts, uint64_t* gcSnapshotBitmap) { + std::lock_guard lock(chainMutex); // Drain the pending retirement slot left by deleteTileRecord's empty-chain path. VersionedData* pending = pendingRetire.exchange(nullptr, std::memory_order_acquire); if (pending) { @@ -493,6 +496,7 @@ template void TileVisibility::exportChainItemsAfter( uint32_t tileId, uint64_t safeGcTs, std::vector>& gcChainItems) const { + std::lock_guard lock(chainMutex); auto* ver = currentVersion.load(std::memory_order_acquire); auto* tailSnap = tail.load(std::memory_order_acquire); size_t tailUsedSnap = tailUsed.load(std::memory_order_acquire); @@ -523,6 +527,7 @@ void TileVisibility::exportChainItemsAfter( template void TileVisibility::importDeletionItems(std::vector& bucket) { + std::lock_guard lock(chainMutex); std::sort(bucket.begin(), bucket.end(), [](uint64_t a, uint64_t b) { return extractTimestamp(a) < extractTimestamp(b); }); diff --git a/cpp/pixels-retina/test/TileVisibilityTest.cpp b/cpp/pixels-retina/test/TileVisibilityTest.cpp index 7994f62e4..3aa736832 100644 --- a/cpp/pixels-retina/test/TileVisibilityTest.cpp +++ b/cpp/pixels-retina/test/TileVisibilityTest.cpp @@ -854,3 +854,29 @@ TEST_F(TileVisibilityCowFoldTest, DuplicateFoldOnAlreadyDeletedRowIsIdempotent) EXPECT_TRUE(bitSet(afterGc, 13)); EXPECT_TRUE(bitSet(afterGc, 14)); } + +// All writers deliberately share a tile and repeatedly cross block boundaries. +TEST_F(TileVisibilityTest, ConcurrentDeletesRemainVisibleBeforeAndAfterGC) { + for (int round = 0; round < 20; ++round) { + TileVisibility tile; + std::vector writers; + std::atomic start{false}; + for (int worker = 0; worker < 8; ++worker) { + writers.emplace_back([&, worker] { + while (!start.load(std::memory_order_acquire)) std::this_thread::yield(); + for (int i = worker; i < RETINA_CAPACITY; i += 8) + tile.deleteTileRecord(i, 100000 + i); + }); + } + start.store(true, std::memory_order_release); + for (auto& writer : writers) writer.join(); + uint64_t bitmap[BITMAP_SIZE] = {0}; + tile.getTileVisibilityBitmap(UINT64_MAX, bitmap); + for (int i = 0; i < RETINA_CAPACITY; ++i) + ASSERT_TRUE(GET_BITMAP_BIT(bitmap, i)) << "before GC round=" << round << " row=" << i; + tile.collectTileGarbage(200000, bitmap); + tile.getTileVisibilityBitmap(UINT64_MAX, bitmap); + for (int i = 0; i < RETINA_CAPACITY; ++i) + ASSERT_TRUE(GET_BITMAP_BIT(bitmap, i)) << "after GC round=" << round << " row=" << i; + } +} From a294fdf080c44149c0fd61ad2917a25a1448439a Mon Sep 17 00:00:00 2001 From: AntiO2 Date: Sun, 11 Oct 2026 17:47:48 +0800 Subject: [PATCH 2/4] test(retina): expose out-of-order deletion snapshot and GC loss --- cpp/pixels-retina/test/TileVisibilityTest.cpp | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/cpp/pixels-retina/test/TileVisibilityTest.cpp b/cpp/pixels-retina/test/TileVisibilityTest.cpp index 3aa736832..e9e071bc3 100644 --- a/cpp/pixels-retina/test/TileVisibilityTest.cpp +++ b/cpp/pixels-retina/test/TileVisibilityTest.cpp @@ -856,6 +856,31 @@ TEST_F(TileVisibilityCowFoldTest, DuplicateFoldOnAlreadyDeletedRowIsIdempotent) } // All writers deliberately share a tile and repeatedly cross block boundaries. +TEST_F(TileVisibilityTest, OutOfOrderDeletesRespectSnapshotTimestamp) { + v->deleteTileRecord(1, 500); + v->deleteTileRecord(2, 100); + uint64_t bitmap[BITMAP_SIZE] = {0}; + v->getTileVisibilityBitmap(200, bitmap); + EXPECT_FALSE(GET_BITMAP_BIT(bitmap, 1)); + EXPECT_TRUE(GET_BITMAP_BIT(bitmap, 2)); +} + +TEST_F(TileVisibilityTest, OutOfOrderDeletesRespectGcBoundary) { + v->deleteTileRecord(1, 500); + v->deleteTileRecord(2, 100); + uint64_t bitmap[BITMAP_SIZE] = {0}; + v->collectTileGarbage(200, bitmap); + EXPECT_FALSE(GET_BITMAP_BIT(bitmap, 1)); + EXPECT_TRUE(GET_BITMAP_BIT(bitmap, 2)); + std::vector> futureDeletes; + v->exportChainItemsAfter(0, 200, futureDeletes); + ASSERT_EQ(futureDeletes.size(), 1); + EXPECT_EQ(futureDeletes[0], std::make_pair(uint32_t{1}, uint64_t{500})); + v->getTileVisibilityBitmap(600, bitmap); + EXPECT_TRUE(GET_BITMAP_BIT(bitmap, 1)); + EXPECT_TRUE(GET_BITMAP_BIT(bitmap, 2)); +} + TEST_F(TileVisibilityTest, ConcurrentDeletesRemainVisibleBeforeAndAfterGC) { for (int round = 0; round < 20; ++round) { TileVisibility tile; From f7a2bd60059dc86ec2eb801232d6c53f07079477 Mon Sep 17 00:00:00 2001 From: AntiO2 Date: Sun, 11 Oct 2026 17:59:07 +0800 Subject: [PATCH 3/4] fix(retina): publish immutable deletion states without a tile mutex --- cpp/pixels-retina/include/EpochManager.h | 3 + cpp/pixels-retina/include/TileVisibility.h | 98 +-- cpp/pixels-retina/lib/EpochManager.cpp | 19 +- cpp/pixels-retina/lib/TileVisibility.cpp | 632 ++++++------------ cpp/pixels-retina/test/TileVisibilityTest.cpp | 65 +- 5 files changed, 334 insertions(+), 483 deletions(-) diff --git a/cpp/pixels-retina/include/EpochManager.h b/cpp/pixels-retina/include/EpochManager.h index 48c1266b1..d81d06106 100644 --- a/cpp/pixels-retina/include/EpochManager.h +++ b/cpp/pixels-retina/include/EpochManager.h @@ -63,6 +63,9 @@ class EpochManager : public pixels::RetinaBase { */ bool canReclaim(uint64_t retireEpoch) const; + // One scan supplies the exclusive reclaim boundary for a whole batch. + uint64_t oldestActiveEpoch() const; + /** * Get the current global epoch. */ diff --git a/cpp/pixels-retina/include/TileVisibility.h b/cpp/pixels-retina/include/TileVisibility.h index b3f168345..9226bcc3a 100644 --- a/cpp/pixels-retina/include/TileVisibility.h +++ b/cpp/pixels-retina/include/TileVisibility.h @@ -29,7 +29,8 @@ #endif #include "RetinaBase.h" #include -#include +#include +#include #include #include #include @@ -55,9 +56,8 @@ inline uint64_t extractTimestamp(uint64_t raw) { * NORMAL is the live append path: the caller provides a current delete * timestamp and the record is appended to the chain. VERSIONED is used when * replay may race with READY readers; historical deletes publish a new - * VersionedData with a folded baseBitmap. EXCLUSIVE is used only while recovery - * blocks readers and GC; historical deletes may update baseBitmap in place, but - * concurrent recovery writers still need tile-level synchronization. + * VersionedData with a folded baseBitmap. EXCLUSIVE is used during recovery. Both replay modes publish historical + * bitmap folds with CAS, so concurrent recovery writers do not lose updates. */ enum class ReplayMode : uint8_t { NORMAL = 0, @@ -65,45 +65,58 @@ enum class ReplayMode : uint8_t { EXCLUSIVE = 2 }; +// Blocks are immutable after publication. Only a private, unpublished block may +// be filled or linked. The newest block is at the head of the persistent chain. struct DeleteIndexBlock : public pixels::RetinaBase { static constexpr size_t BLOCK_CAPACITY = 8; uint64_t items[BLOCK_CAPACITY] = {0}; - std::atomic next{nullptr}; + size_t count = 0; + DeleteIndexBlock* next = nullptr; }; -/** - * VersionedData - A versioned snapshot of the base state - * Used for Copy-on-Write during garbage collection - * IMPORTANT: head is part of the version to ensure atomic visibility - */ template struct VersionedData : public pixels::RetinaBase> { static constexpr size_t NUM_WORDS = BITMAP_WORDS(CAPACITY); - uint64_t baseBitmap[NUM_WORDS]; + struct Bitmap : public pixels::RetinaBase { + std::array words; + uint64_t* data() { return words.data(); } + const uint64_t* data() const { return words.data(); } + void fill(uint64_t value) { words.fill(value); } + }; + // Ordinary appends share the immutable bitmap rather than copying it. + std::shared_ptr base; + const uint64_t* baseBitmap; uint64_t baseTimestamp; - DeleteIndexBlock* head; // Delete chain head, part of the version + DeleteIndexBlock* head; - // timestamp defaults to 0; bitmap defaults to all-zeros. - explicit VersionedData(uint64_t ts = 0, const uint64_t* bitmap = nullptr, DeleteIndexBlock* h = nullptr) - : baseTimestamp(ts), head(h) { - if (bitmap) - std::memcpy(baseBitmap, bitmap, NUM_WORDS * sizeof(uint64_t)); - else - std::memset(baseBitmap, 0, sizeof(baseBitmap)); + explicit VersionedData(uint64_t ts = 0, const uint64_t* bitmap = nullptr, + DeleteIndexBlock* h = nullptr) + : base(makeBitmap(bitmap)), baseBitmap(base->data()), baseTimestamp(ts), head(h) {} + + VersionedData(const VersionedData& old, DeleteIndexBlock* h) + : base(old.base), baseBitmap(base->data()), baseTimestamp(old.baseTimestamp), head(h) {} + +private: + static std::shared_ptr makeBitmap(const uint64_t* bitmap) { + auto result = std::make_shared(); + if (bitmap) std::memcpy(result->data(), bitmap, NUM_WORDS * sizeof(uint64_t)); + else result->fill(0); + return result; } }; -/** - * RetiredVersion - Tracks a retired version for epoch-based reclamation - */ template struct RetiredVersion : public pixels::RetinaBase> { - VersionedData* data; // Fixed: added + VersionedData* data; DeleteIndexBlock* blocksToDelete; + // A copied head retires one block; a successful GC retires the entire old + // chain, which shares no blocks with its rebuilt replacement. + bool wholeChain; uint64_t retireEpoch; + RetiredVersion* next = nullptr; - RetiredVersion(VersionedData* d, DeleteIndexBlock* b, uint64_t e) - : data(d), blocksToDelete(b), retireEpoch(e) {} + RetiredVersion(VersionedData* d, DeleteIndexBlock* b, bool all, uint64_t e) + : data(d), blocksToDelete(b), wholeChain(all), retireEpoch(e) {} }; template @@ -134,28 +147,21 @@ class TileVisibility : public pixels::RetinaBase> { // EXCLUSIVE: recovery replay without readers; historical deletes fold in place. void deleteTileRecordExclusive(uint16_t rowId, uint64_t ts); - // Publish the chain head, tail, and used slots as one tile-local operation. - // Readers must not observe a reserved slot before its item is initialized. - mutable std::mutex chainMutex; + void retireVersion(VersionedData* version, + DeleteIndexBlock* blocks, bool wholeChain); + static void deleteChain(DeleteIndexBlock* head); + static DeleteIndexBlock* buildChain(const std::vector& items, + DeleteIndexBlock* next = nullptr); + + // The successful CAS is the publication point for bitmap, timestamp, and + // complete chain together. Readers and writers protect old states by epoch. std::atomic*> currentVersion; - std::atomic tail; - std::atomic tailUsed; - - // Retired versions awaiting epoch-based reclamation. Only the GC thread - // (collectTileGarbage / reclaimRetiredVersions) reads and writes this vector, - // so no locking is needed. - std::vector> retired; - - // Lock-free staging slot between deleteTileRecord (CDC threads) and GC. - // deleteTileRecord's empty-chain path replaces currentVersion but cannot - // write `retired` directly — that would race with the GC thread. Instead - // it atomically stores oldVer here. The GC thread drains this slot at the - // start of collectTileGarbage, moving it into `retired` with a proper epoch. - // Flow: deleteTileRecord → pendingRetire.store → collectTileGarbage → - // pendingRetire.exchange(nullptr) → retired.emplace_back → reclaimRetiredVersions - // At most one version is pending per GC cycle (the empty-chain path fires - // at most once between consecutive GC compactions). - std::atomic*> pendingRetire{nullptr}; + std::atomic*> pendingRetire{nullptr}; + std::atomic retireCount{0}; + // This try-only guard coordinates reclamation bookkeeping, never reads or + // publication. A busy collector is skipped; no operation waits for it. + std::atomic_flag reclaiming = ATOMIC_FLAG_INIT; + std::vector*> retired; }; #endif // PIXELS_RETINA_TILE_VISIBILITY_H diff --git a/cpp/pixels-retina/lib/EpochManager.cpp b/cpp/pixels-retina/lib/EpochManager.cpp index bfe583f46..615651c1f 100644 --- a/cpp/pixels-retina/lib/EpochManager.cpp +++ b/cpp/pixels-retina/lib/EpochManager.cpp @@ -54,6 +54,9 @@ void EpochManager::enterEpoch() { ThreadInfo* info = getOrCreateThreadInfo(); uint64_t currentEpoch = globalEpoch.load(std::memory_order_acquire); info->localEpoch.store(currentEpoch, std::memory_order_release); + // Publish participation before loading any epoch-protected pointer. Paired + // with the collector fence, this prevents both sides missing each other. + std::atomic_thread_fence(std::memory_order_seq_cst); } void EpochManager::exitEpoch() { @@ -67,7 +70,15 @@ uint64_t EpochManager::advanceEpoch() { } bool EpochManager::canReclaim(uint64_t retireEpoch) const { + return retireEpoch < oldestActiveEpoch(); +} + +uint64_t EpochManager::oldestActiveEpoch() const { + // The retired state was unpublished before this scan. A reader either + // observes the replacement or its epoch announcement must be visible. + std::atomic_thread_fence(std::memory_order_seq_cst); // Scan all threads to find the minimum active epoch + uint64_t oldest = globalEpoch.load(std::memory_order_acquire) + 1; ThreadInfo* current = threadListHead.load(std::memory_order_acquire); while (current) { @@ -79,14 +90,10 @@ bool EpochManager::canReclaim(uint64_t retireEpoch) const { uint64_t localEpoch = current->localEpoch.load(std::memory_order_acquire); // localEpoch == 0 means the thread is not in critical section, skip it - if (localEpoch != 0 && localEpoch <= retireEpoch) { - // Found a thread still in or before the retire epoch - return false; - } + if (localEpoch != 0 && localEpoch < oldest) oldest = localEpoch; current = current->next; } - // All threads have advanced past the retire epoch - return true; + return oldest; } diff --git a/cpp/pixels-retina/lib/TileVisibility.cpp b/cpp/pixels-retina/lib/TileVisibility.cpp index 775238653..bee73022f 100644 --- a/cpp/pixels-retina/lib/TileVisibility.cpp +++ b/cpp/pixels-retina/lib/TileVisibility.cpp @@ -27,6 +27,8 @@ #include #include +constexpr size_t DeleteIndexBlock::BLOCK_CAPACITY; + #if defined(RETINA_SIMD) && (defined(__x86_64__) || defined(__i386__)) #include #define RETINA_X86_AVX2 1 @@ -38,176 +40,115 @@ template TileVisibility::TileVisibility(uint64_t timestamp, const uint64_t* bitmap) - : currentVersion(new VersionedData(timestamp, bitmap)), - tail(nullptr), tailUsed(0) {} + : currentVersion(new VersionedData(timestamp, bitmap)) {} template -TileVisibility::~TileVisibility() { - VersionedData* ver = currentVersion.load(std::memory_order_acquire); - if (ver) { - DeleteIndexBlock *blk = ver->head; - while (blk) { - DeleteIndexBlock *next = blk->next.load(std::memory_order_acquire); - delete blk; - blk = next; - } - delete ver; +void TileVisibility::deleteChain(DeleteIndexBlock* blk) { + while (blk) { + auto* next = blk->next; + delete blk; + blk = next; } +} - // Clean up any version left in the pending retirement slot - VersionedData* pending = pendingRetire.load(std::memory_order_acquire); - if (pending) { - delete pending; +template +TileVisibility::~TileVisibility() { + // Destruction requires callers to have stopped using this tile. + auto* current = currentVersion.load(std::memory_order_relaxed); + deleteChain(current->head); + delete current; + auto* pending = pendingRetire.exchange(nullptr); + while (pending) { + auto* next = pending->next; + retired.push_back(pending); + pending = next; } - - // Clean up retired versions and their delete chains - for (auto& retired : this->retired) { - if (retired.data) { - delete retired.data; - } - DeleteIndexBlock* blk = retired.blocksToDelete; - while (blk) { - DeleteIndexBlock* next = blk->next.load(std::memory_order_acquire); - delete blk; - blk = next; - } + for (auto* entry : retired) { + if (entry->wholeChain) deleteChain(entry->blocksToDelete); + else delete entry->blocksToDelete; + delete entry->data; + delete entry; } } template -void TileVisibility::deleteTileRecord(uint16_t rowId, uint64_t ts, - ReplayMode replayMode) { - std::lock_guard lock(chainMutex); - switch (replayMode) { - case ReplayMode::NORMAL: - appendDeleteChain(rowId, ts); - return; - case ReplayMode::VERSIONED: - deleteTileRecordVersioned(rowId, ts); - return; - case ReplayMode::EXCLUSIVE: - deleteTileRecordExclusive(rowId, ts); - return; - default: - throw std::invalid_argument("unknown ReplayMode"); +void TileVisibility::retireVersion(VersionedData* version, + DeleteIndexBlock* blocks, bool wholeChain) { + auto epoch = EpochManager::getInstance().advanceEpoch(); + auto* entry = new RetiredVersion(version, blocks, wholeChain, epoch); + auto* pending = pendingRetire.load(std::memory_order_relaxed); + do { + entry->next = pending; + } while (!pendingRetire.compare_exchange_weak(pending, entry, + std::memory_order_release, std::memory_order_relaxed)); +} + +template +void TileVisibility::deleteTileRecord(uint16_t rowId, uint64_t ts, ReplayMode mode) { + { + EpochGuard guard; + switch (mode) { + case ReplayMode::NORMAL: appendDeleteChain(rowId, ts); break; + case ReplayMode::VERSIONED: deleteTileRecordVersioned(rowId, ts); break; + case ReplayMode::EXCLUSIVE: deleteTileRecordExclusive(rowId, ts); break; + default: throw std::invalid_argument("unknown ReplayMode"); + } } + // Bound obsolete publication states between GC cycles. Reclamation runs + // outside our epoch and does not block a concurrent publisher or reader. + if ((retireCount.fetch_add(1, std::memory_order_relaxed) & 63) == 0) + reclaimRetiredVersions(); } template void TileVisibility::deleteTileRecordVersioned(uint16_t rowId, uint64_t ts) { - // READY backlog replay can race with getTileVisibilityBitmap readers. Fold - // historical deletes by publishing a new VersionedData instead of mutating - // baseBitmap observed by an existing reader. - // Keep ts=0 out of this path because item=0 is the chain-slot sentinel. while (ts > 0) { - VersionedData* cur = currentVersion.load(std::memory_order_acquire); - if (ts > cur->baseTimestamp) { - break; - } - if ((cur->baseBitmap[rowId / 64] & (1ULL << (rowId % 64))) != 0) { + auto* cur = currentVersion.load(std::memory_order_acquire); + if (ts > cur->baseTimestamp) break; + if (cur->baseBitmap[rowId / 64] & (1ULL << (rowId % 64))) return; + uint64_t bitmap[NUM_WORDS]; + std::memcpy(bitmap, cur->baseBitmap, sizeof(bitmap)); + SET_BITMAP_BIT(bitmap, rowId); + auto* replacement = new VersionedData(cur->baseTimestamp, bitmap, cur->head); + if (currentVersion.compare_exchange_strong(cur, replacement, std::memory_order_acq_rel)) { + retireVersion(cur, nullptr, false); return; } - uint64_t newBaseBitmap[NUM_WORDS]; - std::memcpy(newBaseBitmap, cur->baseBitmap, NUM_WORDS * sizeof(uint64_t)); - SET_BITMAP_BIT(newBaseBitmap, rowId); - VersionedData* newVer = - new VersionedData(cur->baseTimestamp, newBaseBitmap, cur->head); - if (currentVersion.compare_exchange_strong(cur, newVer, std::memory_order_acq_rel)) { - pendingRetire.store(cur, std::memory_order_release); - return; - } - delete newVer; + delete replacement; } - appendDeleteChain(rowId, ts); } template void TileVisibility::deleteTileRecordExclusive(uint16_t rowId, uint64_t ts) { - // RECOVERING replay blocks readers and GC, so historical deletes can fold - // into baseBitmap in place. Atomic OR prevents lost updates when concurrent - // recovery writers touch the same bitmap word. - VersionedData* cur = currentVersion.load(std::memory_order_acquire); - if (ts > 0 && ts <= cur->baseTimestamp) { - uint64_t mask = 1ULL << (rowId % 64); - __atomic_fetch_or(&cur->baseBitmap[rowId / 64], mask, __ATOMIC_RELAXED); - return; - } - - appendDeleteChain(rowId, ts); + // Recovery writers also use immutable bitmap publication. In-place atomic + // OR would race with another writer copying the bitmap for a new state. + deleteTileRecordVersioned(rowId, ts); } template void TileVisibility::appendDeleteChain(uint16_t rowId, uint64_t ts) { - // Normal live apply assumes a current timestamp and records the delete in - // the append-only chain, leaving baseBitmap untouched for the hot path. uint64_t item = makeDeleteIndex(rowId, ts); + // Preserve the existing reserved-zero encoding; transaction timestamps are + // nonzero, and (row 0, timestamp 0) is not a deletion record. + if (item == 0) return; while (true) { - DeleteIndexBlock *curTail = tail.load(std::memory_order_acquire); - if (!curTail) { // empty list - need to create first block and update version - auto *newBlk = new DeleteIndexBlock(); - newBlk->items[0] = item; - DeleteIndexBlock *expectedTail = nullptr; - - if (!tail.compare_exchange_strong(expectedTail, newBlk, - std::memory_order_release, - std::memory_order_relaxed)) { - delete newBlk; - continue; - } - - // COW: Create new version with the new head - VersionedData* oldVer = currentVersion.load(std::memory_order_acquire); - VersionedData* newVer = new VersionedData(oldVer->baseTimestamp, oldVer->baseBitmap, newBlk); - - if (currentVersion.compare_exchange_strong(oldVer, newVer, std::memory_order_acq_rel)) { - // Defer retirement: a concurrent reader may still hold oldVer under EpochGuard. - // collectTileGarbage will drain this slot and epoch-retire it properly. - pendingRetire.store(oldVer, std::memory_order_release); - tailUsed.store(1, std::memory_order_release); - return; - } else { - // CAS failed, retry from beginning - delete newVer; - tail.store(nullptr, std::memory_order_release); - delete newBlk; - continue; - } - } else { - size_t pos = tailUsed.load(std::memory_order_acquire); - if (pos < DeleteIndexBlock::BLOCK_CAPACITY) { - if (tailUsed.compare_exchange_strong(pos, pos + 1, - std::memory_order_relaxed, - std::memory_order_relaxed)) { - curTail->items[pos] = item; - return; - } - } else { - // curTail is full, need to add new block - DeleteIndexBlock *newBlk = new DeleteIndexBlock(); - newBlk->items[0] = item; - - if (tail.load(std::memory_order_acquire) != curTail) { - delete newBlk; - continue; - } - - DeleteIndexBlock *expectedNext = nullptr; - if (!curTail->next.compare_exchange_strong( - expectedNext, newBlk, - std::memory_order_release, - std::memory_order_relaxed)) { - delete newBlk; - continue; - } - - tail.compare_exchange_strong(curTail, newBlk, - std::memory_order_release, - std::memory_order_relaxed); - tailUsed.store(1, std::memory_order_release); - return; - } + auto* cur = currentVersion.load(std::memory_order_acquire); + auto* head = new DeleteIndexBlock(); + bool copyHead = cur->head && cur->head->count < DeleteIndexBlock::BLOCK_CAPACITY; + if (copyHead) { + head->count = cur->head->count; + std::memcpy(head->items, cur->head->items, head->count * sizeof(uint64_t)); + head->next = cur->head->next; + } else head->next = cur->head; + head->items[head->count++] = item; + auto* replacement = new VersionedData(*cur, head); + if (currentVersion.compare_exchange_strong(cur, replacement, std::memory_order_acq_rel)) { + retireVersion(cur, copyHead ? cur->head : nullptr, false); + return; } + delete replacement; + delete head; } } @@ -264,20 +205,12 @@ inline void process_bitmap_block_128(const DeleteIndexBlock *blk, #endif template -void TileVisibility::getTileVisibilityBitmap(uint64_t ts, uint64_t* outBitmap) const { - std::lock_guard lock(chainMutex); - // Enter epoch protection +void TileVisibility::getTileVisibilityBitmap(uint64_t ts, uint64_t* bitmap) const { EpochGuard guard; - // Load current version under epoch protection - VersionedData* ver = currentVersion.load(std::memory_order_acquire); - - if (ts < ver->baseTimestamp) { - throw std::runtime_error("need to read checkpoint from disk"); - } - std::memcpy(outBitmap, ver->baseBitmap, NUM_WORDS * sizeof(uint64_t)); + auto* ver = currentVersion.load(std::memory_order_acquire); + if (ts < ver->baseTimestamp) throw std::runtime_error("need to read checkpoint from disk"); + std::memcpy(bitmap, ver->baseBitmap, NUM_WORDS * sizeof(uint64_t)); if (ts == ver->baseTimestamp) return; - - DeleteIndexBlock *blk = ver->head; #if defined(RETINA_X86_AVX2) const __m256i signBit = _mm256_set1_epi64x(0x8000000000000000ULL); const __m256i vThrFlip = _mm256_xor_si256(_mm256_set1_epi64x(ts), signBit); @@ -286,316 +219,155 @@ void TileVisibility::getTileVisibilityBitmap(uint64_t ts, uint64_t* ou const uint64x2_t vThr = vdupq_n_u64(ts); const uint64x2_t tsMask = vdupq_n_u64(0x0000FFFFFFFFFFFFULL); #endif - - while (blk) { - DeleteIndexBlock *currentTail = tail.load(std::memory_order_relaxed); - size_t currentTailUsed = tailUsed.load(std::memory_order_relaxed); - size_t count = (blk == currentTail) ? currentTailUsed : DeleteIndexBlock::BLOCK_CAPACITY; - - // Same tail/tailUsed race as in collectTileGarbage: count may be 0 or - // a stale BLOCK_CAPACITY for a newly-created tail block. count == 0 - // means no items to read; skip cleanly. The stale-count case (items - // beyond the first being zero-initialised) is handled in the scalar - // path below via the item == 0 sentinel check. - if (count == 0) { - blk = blk->next.load(std::memory_order_relaxed); - continue; - } - - uint64_t i = 0; + for (auto* blk = ver->head; blk; blk = blk->next) { + size_t i = 0; #if defined(RETINA_X86_AVX2) - // NOTE: the SIMD path does not check for zero-initialised (item == 0) - // sentinel values. In the extremely rare stale-tailUsed race window, - // up to BLOCK_CAPACITY-1 zero items may cause row 0 to be transiently - // marked as deleted in the output bitmap. This is a known limitation - // of the SIMD fast path; the effect is transient (not persisted) and - // self-correcting on the next query once tailUsed is fully updated. - for (; i + 4 <= count; i += 4) { - process_bitmap_block_256(blk, i, outBitmap, vThrFlip, tsMask, signBit); - } + for (; i + 4 <= blk->count; i += 4) + process_bitmap_block_256(blk, i, bitmap, vThrFlip, tsMask, signBit); #elif defined(RETINA_ARM_NEON) - for (; i + 2 <= count; i += 2) { - process_bitmap_block_128(blk, i, outBitmap, vThr, tsMask); - } + for (; i + 2 <= blk->count; i += 2) + process_bitmap_block_128(blk, i, bitmap, vThr, tsMask); #endif - for (; i < count; i++) { - uint64_t item = blk->items[i]; - // Sentinel: zero item signals an uninitialised slot (see - // collectTileGarbage for the full race description). - if (item == 0) return; - if (extractTimestamp(item) <= ts) { - SET_BITMAP_BIT(outBitmap, extractRowId(item)); - } else { - return; - } + for (; i < blk->count; ++i) { + auto item = blk->items[i]; + if (extractTimestamp(item) <= ts) SET_BITMAP_BIT(bitmap, extractRowId(item)); } - blk = blk->next.load(std::memory_order_relaxed); } } template -void TileVisibility::collectTileGarbage(uint64_t ts, uint64_t* gcSnapshotBitmap) { - std::lock_guard lock(chainMutex); - // Drain the pending retirement slot left by deleteTileRecord's empty-chain path. - VersionedData* pending = pendingRetire.exchange(nullptr, std::memory_order_acquire); - if (pending) { - uint64_t retireEpoch = EpochManager::getInstance().advanceEpoch(); - retired.emplace_back(pending, nullptr, retireEpoch); - } - - // Load old version - VersionedData* oldVer = currentVersion.load(std::memory_order_acquire); - - // Early return A: safeGcTs <= baseTimestamp, nothing to compact - if (ts <= oldVer->baseTimestamp) { - std::memcpy(gcSnapshotBitmap, oldVer->baseBitmap, NUM_WORDS * sizeof(uint64_t)); - return; - } - - // Find the last block that should be compacted. - // Snapshot tail/tailUsed once and reuse in both the scan loop and the - // compact loop to guarantee a consistent view of the chain endpoint. - DeleteIndexBlock *blk = oldVer->head; - DeleteIndexBlock *lastFullBlk = nullptr; - uint64_t newBaseTimestamp = oldVer->baseTimestamp; - auto* tailSnap1 = tail.load(std::memory_order_acquire); - size_t tailUsedSnap1 = tailUsed.load(std::memory_order_acquire); - - while (blk) { - size_t count = (blk == tailSnap1) - ? tailUsedSnap1 - : DeleteIndexBlock::BLOCK_CAPACITY; - // Guard: deleteTileRecord updates `tail` and `tailUsed` non-atomically. - // In the narrow window after `tail` is advanced to a new block but before - // `tailUsed.store(1)` completes, we may observe count == 0 (empty-list - // path: tailUsed transitions 0 → 1) or a stale BLOCK_CAPACITY (full-block - // path: tailUsed transitions BLOCK_CAPACITY → 1 via store, not CAS). - // When count == 0 there is nothing to compact; stop here and let the next - // GC cycle handle the block once it is fully initialised. - if (count == 0) break; - uint64_t lastItemTs = extractTimestamp(blk->items[count - 1]); - if (lastItemTs <= ts) { - lastFullBlk = blk; - newBaseTimestamp = lastItemTs; - } else break; - blk = blk->next.load(std::memory_order_acquire); +DeleteIndexBlock* TileVisibility::buildChain(const std::vector& items, + DeleteIndexBlock* next) { + auto* head = next; + // Each block is completely initialized before it becomes reachable. + for (size_t offset = 0; offset < items.size(); offset += DeleteIndexBlock::BLOCK_CAPACITY) { + auto* block = new DeleteIndexBlock(); + block->count = std::min(DeleteIndexBlock::BLOCK_CAPACITY, items.size() - offset); + std::memcpy(block->items, items.data() + offset, block->count * sizeof(uint64_t)); + block->next = head; + head = block; } + return head; +} - // Early return B: no compactable block - if (!lastFullBlk) { - std::memcpy(gcSnapshotBitmap, oldVer->baseBitmap, NUM_WORDS * sizeof(uint64_t)); - if (oldVer->head) { - auto* tailSnap = tail.load(std::memory_order_acquire); - size_t tailUsedSnap = tailUsed.load(std::memory_order_acquire); - size_t cnt = (oldVer->head == tailSnap) ? tailUsedSnap : DeleteIndexBlock::BLOCK_CAPACITY; - for (size_t i = 0; i < cnt; i++) { - uint64_t item = oldVer->head->items[i]; - if (item == 0) break; - if (extractTimestamp(item) <= ts) SET_BITMAP_BIT(gcSnapshotBitmap, extractRowId(item)); - else break; +template +void TileVisibility::collectTileGarbage(uint64_t ts, uint64_t* snapshot) { + { + EpochGuard guard; + while (true) { + auto* old = currentVersion.load(std::memory_order_acquire); + std::memcpy(snapshot, old->baseBitmap, NUM_WORDS * sizeof(uint64_t)); + if (ts <= old->baseTimestamp) break; + uint64_t baseTs = old->baseTimestamp; + std::vector future; + bool folded = false; + for (auto* block = old->head; block; block = block->next) { + for (size_t i = 0; i < block->count; ++i) { + auto item = block->items[i]; + auto itemTs = extractTimestamp(item); + if (itemTs <= ts) { + SET_BITMAP_BIT(snapshot, extractRowId(item)); + baseTs = std::max(baseTs, itemTs); + folded = true; + } else future.push_back(item); + } } - } - return; - } - - // Create new version with Copy-on-Write - // Manually compute the new base bitmap from oldVer - uint64_t newBaseBitmap[NUM_WORDS]; - std::memcpy(newBaseBitmap, oldVer->baseBitmap, NUM_WORDS * sizeof(uint64_t)); - - // Apply deletes from oldVer->head up to lastFullBlk - blk = oldVer->head; - while (blk) { - size_t count = (blk == lastFullBlk && blk == tailSnap1) ? tailUsedSnap1 : DeleteIndexBlock::BLOCK_CAPACITY; - for (size_t i = 0; i < count; i++) { - uint64_t item = blk->items[i]; - // Guard: a zero item means an uninitialised slot in a newly-created - // tail block observed under the same tail/tailUsed race described - // above (full-block path: tailUsed is still BLOCK_CAPACITY while - // only items[0] is valid; items[1..n] remain zero-initialised). - // item == 0 encodes makeDeleteIndex(rowId=0, ts=0); since all valid - // transaction timestamps are > 0, this value is never a legitimate - // deletion record and safely identifies the end of valid items. - if (item == 0) break; - if (extractTimestamp(item) <= ts) { - SET_BITMAP_BIT(newBaseBitmap, extractRowId(item)); + if (!folded) break; + auto* head = buildChain(future); + auto* replacement = new VersionedData(baseTs, snapshot, head); + if (currentVersion.compare_exchange_strong(old, replacement, std::memory_order_acq_rel)) { + // No published links were changed. Readers of old keep their + // full chain, and future deletes remain in the replacement. + retireVersion(old, old->head, true); + break; } - } - if (blk == lastFullBlk) break; - blk = blk->next.load(std::memory_order_acquire); - } - - // Compact path: build gcSnapshotBitmap by scanning the boundary block. - // Reuse the same tail/tailUsed snapshot (tailSnap1/tailUsedSnap1) taken at - // the start of this GC cycle to ensure consistent chain-end semantics. - DeleteIndexBlock* newHead = lastFullBlk->next.load(std::memory_order_acquire); - std::memcpy(gcSnapshotBitmap, newBaseBitmap, NUM_WORDS * sizeof(uint64_t)); - if (newHead) { - size_t cnt = (newHead == tailSnap1) ? tailUsedSnap1 : DeleteIndexBlock::BLOCK_CAPACITY; - for (size_t i = 0; i < cnt; i++) { - uint64_t item = newHead->items[i]; - if (item == 0) break; - if (extractTimestamp(item) <= ts) SET_BITMAP_BIT(gcSnapshotBitmap, extractRowId(item)); - else break; + delete replacement; + deleteChain(head); } } - - // Break the chain to avoid double-free - lastFullBlk->next.store(nullptr, std::memory_order_release); - - // Create new version with new head - this is the atomic COW update - VersionedData* newVer = new VersionedData(newBaseTimestamp, newBaseBitmap, newHead); - - // CAS to install new version atomically - if (currentVersion.compare_exchange_strong(oldVer, newVer, std::memory_order_acq_rel)) { - // Successfully updated - // Retire old version and its delete chain - uint64_t retireEpoch = EpochManager::getInstance().advanceEpoch(); - retired.emplace_back(oldVer, oldVer->head, retireEpoch); - - // Update tail if needed (if all blocks were compacted) - if (!newHead) { - tail.store(nullptr, std::memory_order_release); - tailUsed.store(0, std::memory_order_release); - } - - // Try to reclaim retired versions - reclaimRetiredVersions(); - } else { - // CAS failed, another GC happened concurrently - // Restore the chain link - lastFullBlk->next.store(newHead, std::memory_order_release); - delete newVer; - } + reclaimRetiredVersions(); } template void TileVisibility::reclaimRetiredVersions() { - auto it = retired.begin(); - while (it != retired.end()) { - if (EpochManager::getInstance().canReclaim(it->retireEpoch)) { - // Safe to delete - if (it->data) { - delete it->data; - } - // Delete the chain of blocks - DeleteIndexBlock* blk = it->blocksToDelete; - while (blk) { - DeleteIndexBlock* next = blk->next.load(std::memory_order_acquire); - delete blk; - blk = next; - } - it = retired.erase(it); - } else { - ++it; - } + if (reclaiming.test_and_set(std::memory_order_acquire)) return; + auto* pending = pendingRetire.exchange(nullptr, std::memory_order_acquire); + while (pending) { + auto* next = pending->next; + retired.push_back(pending); + pending = next; } + // Reclaim old entries even while other writers continue to publish. Waiting + // for the newest entry would require a global quiescent gap under load. + const auto boundary = EpochManager::getInstance().oldestActiveEpoch(); + size_t keep = 0; + for (auto* entry : retired) { + if (entry->retireEpoch < boundary) { + if (entry->wholeChain) deleteChain(entry->blocksToDelete); + else delete entry->blocksToDelete; + delete entry->data; + delete entry; + } else retired[keep++] = entry; + } + retired.resize(keep); + reclaiming.clear(std::memory_order_release); } template -void TileVisibility::exportChainItemsAfter( - uint32_t tileId, uint64_t safeGcTs, - std::vector>& gcChainItems) const { - std::lock_guard lock(chainMutex); +void TileVisibility::exportChainItemsAfter(uint32_t tileId, uint64_t safeGcTs, + std::vector>& output) const { + EpochGuard guard; auto* ver = currentVersion.load(std::memory_order_acquire); - auto* tailSnap = tail.load(std::memory_order_acquire); - size_t tailUsedSnap = tailUsed.load(std::memory_order_acquire); - - auto* blk = ver->head; - bool pastBoundary = false; - while (blk != nullptr) { - size_t count = (blk == tailSnap) ? tailUsedSnap : DeleteIndexBlock::BLOCK_CAPACITY; - for (size_t i = 0; i < count; i++) { - uint64_t item = blk->items[i]; - if (item == 0) return; - if (pastBoundary) { - uint32_t rgOffset = tileId * CAPACITY + extractRowId(item); - gcChainItems.push_back({rgOffset, extractTimestamp(item)}); - } else { - uint64_t ts = extractTimestamp(item); - if (ts > safeGcTs) { - pastBoundary = true; - uint32_t rgOffset = tileId * CAPACITY + extractRowId(item); - gcChainItems.push_back({rgOffset, ts}); - } - } + const size_t start = output.size(); + for (auto* block = ver->head; block; block = block->next) { + for (size_t i = 0; i < block->count; ++i) { + auto item = block->items[i]; + auto ts = extractTimestamp(item); + if (ts > safeGcTs) output.emplace_back(tileId * CAPACITY + extractRowId(item), ts); } - if (blk == tailSnap) return; - blk = blk->next.load(std::memory_order_acquire); } + // Preserve the checkpoint API's chronological output without requiring CDC + // threads to append in timestamp order. + std::stable_sort(output.begin() + start, output.end(), [](const std::pair& a, + const std::pair& b) { + return a.second < b.second; + }); } template void TileVisibility::importDeletionItems(std::vector& bucket) { - std::lock_guard lock(chainMutex); - std::sort(bucket.begin(), bucket.end(), [](uint64_t a, uint64_t b) { - return extractTimestamp(a) < extractTimestamp(b); - }); - - bool tailClaimed = false; - while (true) { - auto* ver = currentVersion.load(std::memory_order_acquire); - - uint64_t ts_head = UINT64_MAX; - if (ver->head != nullptr) { - uint64_t firstItem = ver->head->items[0]; - if (firstItem != 0) ts_head = extractTimestamp(firstItem); - } - - size_t keepCount = bucket.size(); - if (ts_head != UINT64_MAX) { - keepCount = std::upper_bound(bucket.begin(), bucket.end(), ts_head, - [](uint64_t val, uint64_t item) { - return val < extractTimestamp(item); - }) - bucket.begin(); - } - if (keepCount == 0) return; - - uint64_t lastValidItem = bucket[keepCount - 1]; - std::vector blocks; - for (size_t i = 0; i < keepCount; i += DeleteIndexBlock::BLOCK_CAPACITY) { - auto* blk = new DeleteIndexBlock(); - for (size_t j = 0; j < DeleteIndexBlock::BLOCK_CAPACITY; j++) { - size_t idx = i + j; - blk->items[j] = (idx < keepCount) ? bucket[idx] : lastValidItem; + if (bucket.empty()) return; + { + EpochGuard guard; + while (true) { + auto* old = currentVersion.load(std::memory_order_acquire); + uint64_t bitmap[NUM_WORDS]; + std::memcpy(bitmap, old->baseBitmap, sizeof(bitmap)); + std::vector future; + for (auto item : bucket) { + if (extractTimestamp(item) <= old->baseTimestamp) + SET_BITMAP_BIT(bitmap, extractRowId(item)); + else future.push_back(item); } - blocks.push_back(blk); - } - for (size_t i = 0; i + 1 < blocks.size(); i++) - blocks[i]->next.store(blocks[i + 1], std::memory_order_release); - blocks.back()->next.store(ver->head, std::memory_order_release); - - if (ver->head == nullptr && !tailClaimed) { - size_t lastBlockItems = keepCount % DeleteIndexBlock::BLOCK_CAPACITY; - if (lastBlockItems == 0) lastBlockItems = DeleteIndexBlock::BLOCK_CAPACITY; - - DeleteIndexBlock* expectedTail = nullptr; - if (tail.compare_exchange_strong(expectedTail, blocks.back(), - std::memory_order_release, std::memory_order_relaxed)) { - tailUsed.store(lastBlockItems, std::memory_order_release); - tailClaimed = true; - } else { - for (auto* blk : blocks) delete blk; - continue; + // Different rows at the same/later timestamp are not duplicates. + // Replaying a duplicate is harmless for the deletion bitmap. + auto* oldHead = old->head; + auto* head = buildChain(future, oldHead); + auto* replacement = new VersionedData(old->baseTimestamp, bitmap, head); + if (currentVersion.compare_exchange_strong(old, replacement, std::memory_order_acq_rel)) { + retireVersion(old, nullptr, false); + break; + } + delete replacement; + // Only our unpublished prefix belongs to this failed attempt. + while (head != oldHead) { + auto* next = head->next; + delete head; + head = next; } } - - auto* newVer = new VersionedData(ver->baseTimestamp, ver->baseBitmap, blocks[0]); - - if (currentVersion.compare_exchange_strong(ver, newVer, std::memory_order_acq_rel)) { - uint64_t retireEpoch = EpochManager::getInstance().advanceEpoch(); - retired.emplace_back(ver, nullptr, retireEpoch); - reclaimRetiredVersions(); - return; - } - if (tailClaimed) { - std::fprintf(stderr, "importDeletionItems: CAS failed with tailClaimed — invariant violation\n"); - std::abort(); - } - delete newVer; - for (auto* blk : blocks) delete blk; } + reclaimRetiredVersions(); } -// Explicit Instantiations (Add the sizes you need here) -template class TileVisibility; \ No newline at end of file +template class TileVisibility; diff --git a/cpp/pixels-retina/test/TileVisibilityTest.cpp b/cpp/pixels-retina/test/TileVisibilityTest.cpp index e9e071bc3..bff75d264 100644 --- a/cpp/pixels-retina/test/TileVisibilityTest.cpp +++ b/cpp/pixels-retina/test/TileVisibilityTest.cpp @@ -652,7 +652,7 @@ TEST_F(TileVisibilityTest, ImportDeletionItems_TruncationDedup) { EXPECT_TRUE(checkBitmap(actualBitmap, expectedBitmap)); } -TEST_F(TileVisibilityTest, ImportDeletionItems_FullOverlap) { +TEST_F(TileVisibilityTest, ImportDeletionItems_SameTimestampsDifferentRows) { v->deleteTileRecord(20, 100); v->deleteTileRecord(21, 200); @@ -663,6 +663,8 @@ TEST_F(TileVisibilityTest, ImportDeletionItems_FullOverlap) { v->importDeletionItems(bucket); uint64_t expectedBitmap[BITMAP_SIZE] = {0}; + SET_BITMAP_BIT(expectedBitmap, 1); + SET_BITMAP_BIT(expectedBitmap, 2); SET_BITMAP_BIT(expectedBitmap, 20); SET_BITMAP_BIT(expectedBitmap, 21); @@ -905,3 +907,64 @@ TEST_F(TileVisibilityTest, ConcurrentDeletesRemainVisibleBeforeAndAfterGC) { ASSERT_TRUE(GET_BITMAP_BIT(bitmap, i)) << "after GC round=" << round << " row=" << i; } } + +TEST_F(TileVisibilityTest, ConcurrentAppendImportReadAndPartialGC) { + for (int round = 0; round < 10; ++round) { + TileVisibility tile; + std::vector> completed(RETINA_CAPACITY); + for (auto& flag : completed) flag.store(false); + std::atomic start{false}, stop{false}, missed{false}; + std::atomic gcCount{0}; + std::thread collector([&] { + while (!start.load(std::memory_order_acquire)) std::this_thread::yield(); + while (!stop.load(std::memory_order_acquire)) { + uint64_t bitmap[BITMAP_SIZE] = {0}; + tile.collectTileGarbage(200, bitmap); + gcCount.fetch_add(1); + std::this_thread::yield(); + } + }); + std::thread reader([&] { + while (!start.load(std::memory_order_acquire)) std::this_thread::yield(); + while (!stop.load(std::memory_order_acquire)) { + std::vector known(RETINA_CAPACITY); + for (int i = 0; i < RETINA_CAPACITY; ++i) + known[i] = completed[i].load(std::memory_order_acquire); + uint64_t bitmap[BITMAP_SIZE] = {0}; + tile.getTileVisibilityBitmap(600, bitmap); + for (int i = 0; i < RETINA_CAPACITY; ++i) + if (known[i] && !GET_BITMAP_BIT(bitmap, i)) missed.store(true); + } + }); + std::vector writers; + for (int worker = 0; worker < 8; ++worker) { + writers.emplace_back([&, worker] { + while (!start.load(std::memory_order_acquire)) std::this_thread::yield(); + for (int i = worker; i < RETINA_CAPACITY; i += 8) { + uint64_t ts = (i % 2) ? 100 : 500; + if (worker < 4) tile.deleteTileRecord(i, ts); + else { + std::vector items{makeDeleteIndex(i, ts)}; + tile.importDeletionItems(items); + } + completed[i].store(true, std::memory_order_release); + } + }); + } + start.store(true, std::memory_order_release); + for (auto& writer : writers) writer.join(); + stop.store(true, std::memory_order_release); + collector.join(); + reader.join(); + ASSERT_GT(gcCount.load(), 0u); + ASSERT_FALSE(missed.load()) << "round=" << round; + uint64_t bitmap[BITMAP_SIZE] = {0}; + tile.getTileVisibilityBitmap(600, bitmap); + for (int i = 0; i < RETINA_CAPACITY; ++i) + ASSERT_TRUE(GET_BITMAP_BIT(bitmap, i)) << "round=" << round << " row=" << i; + tile.collectTileGarbage(600, bitmap); + tile.getTileVisibilityBitmap(600, bitmap); + for (int i = 0; i < RETINA_CAPACITY; ++i) + ASSERT_TRUE(GET_BITMAP_BIT(bitmap, i)) << "after GC round=" << round << " row=" << i; + } +} From 42abedb793a83795be4c6d50cb42dea4f3ead8c2 Mon Sep 17 00:00:00 2001 From: AntiO2 Date: Sun, 11 Oct 2026 18:09:32 +0800 Subject: [PATCH 4/4] test(retina): reproduce concurrent chain loss with one timestamp --- cpp/pixels-retina/test/TileVisibilityTest.cpp | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/cpp/pixels-retina/test/TileVisibilityTest.cpp b/cpp/pixels-retina/test/TileVisibilityTest.cpp index bff75d264..f6e1c4685 100644 --- a/cpp/pixels-retina/test/TileVisibilityTest.cpp +++ b/cpp/pixels-retina/test/TileVisibilityTest.cpp @@ -908,6 +908,31 @@ TEST_F(TileVisibilityTest, ConcurrentDeletesRemainVisibleBeforeAndAfterGC) { } } +TEST_F(TileVisibilityTest, ConcurrentSameTimestampDeletesRemainVisibleBeforeAndAfterGC) { + for (int round = 0; round < 20; ++round) { + TileVisibility tile; + std::vector writers; + std::atomic start{false}; + for (int worker = 0; worker < 8; ++worker) { + writers.emplace_back([&, worker] { + while (!start.load(std::memory_order_acquire)) std::this_thread::yield(); + for (int i = worker; i < RETINA_CAPACITY; i += 8) + tile.deleteTileRecord(i, 100000); + }); + } + start.store(true, std::memory_order_release); + for (auto& writer : writers) writer.join(); + uint64_t bitmap[BITMAP_SIZE] = {0}; + tile.getTileVisibilityBitmap(UINT64_MAX, bitmap); + for (int i = 0; i < RETINA_CAPACITY; ++i) + ASSERT_TRUE(GET_BITMAP_BIT(bitmap, i)) << "before GC round=" << round << " row=" << i; + tile.collectTileGarbage(200000, bitmap); + tile.getTileVisibilityBitmap(UINT64_MAX, bitmap); + for (int i = 0; i < RETINA_CAPACITY; ++i) + ASSERT_TRUE(GET_BITMAP_BIT(bitmap, i)) << "after GC round=" << round << " row=" << i; + } +} + TEST_F(TileVisibilityTest, ConcurrentAppendImportReadAndPartialGC) { for (int round = 0; round < 10; ++round) { TileVisibility tile;