Commit eb073eff authored by Andrey Kamakin's avatar Andrey Kamakin

Fixed invalid history tiles due to getTileLazy implementation in lock free hash table.

Fixed assert due to thread races in extent manager's updateExtent method.

Ref T8874
parent f46a4bb8
......@@ -101,7 +101,7 @@ inline bool KisTiledExtentManager::Data::remove(qint32 index)
--m_count;
needsUpdateExtent = true;
} else {
KIS_ASSERT_RECOVER_NOOP(0 && "sanity check failed: the tile has already been removed!");
m_buffer[currentIndex].deref();
}
} else {
m_buffer[currentIndex].deref();
......@@ -297,26 +297,32 @@ QRect KisTiledExtentManager::extent() const
void KisTiledExtentManager::updateExtent()
{
QReadLocker cl(&m_colsData.m_extentLock);
QReadLocker rl(&m_rowsData.m_extentLock);
qint32 minX, maxX, minY, maxY;
bool colsEmpty = m_colsData.isEmpty();
bool rowsEmpty = m_rowsData.isEmpty();
KIS_ASSERT_RECOVER_RETURN(colsEmpty == rowsEmpty);
{
QReadLocker cl(&m_colsData.m_extentLock);
if (colsEmpty && rowsEmpty) {
QWriteLocker lock(&m_extentLock);
m_currentExtent = QRect(qint32_MAX, qint32_MAX, 0, 0);
} else {
const qint32 minX = m_colsData.min() * KisTileData::WIDTH;
const qint32 maxPlusOneX = (m_colsData.max() + 1) * KisTileData::WIDTH;
const qint32 minY = m_rowsData.min() * KisTileData::HEIGHT;
const qint32 maxPlusOneY = (m_rowsData.max() + 1) * KisTileData::HEIGHT;
QWriteLocker lock(&m_extentLock);
m_currentExtent =
QRect(minX, minY,
maxPlusOneX - minX,
maxPlusOneY - minY);
if (m_colsData.isEmpty()) {
minX = qint32_MAX;
maxX = 0;
} else {
minX = m_colsData.min() * KisTileData::WIDTH;
maxX = (m_colsData.max() + 1) * KisTileData::WIDTH - minX;
}
}
{
QReadLocker rl(&m_rowsData.m_extentLock);
if (m_rowsData.isEmpty()) {
minY = qint32_MAX;
maxY = 0;
} else {
minY = m_rowsData.min() * KisTileData::HEIGHT;
maxY = (m_rowsData.max() + 1) * KisTileData::HEIGHT - minY;
}
}
QWriteLocker lock(&m_extentLock);
m_currentExtent = QRect(minX, minY, maxX, maxY);
}
......@@ -30,7 +30,7 @@
/**
* This is a template for a hash table that stores tiles (or some other
* objects resembling tiles). Actually, this object should only have
* col()/row() methods and be able to answer setNext()/next() requests to
* col()/row() methods and be able to answer notifyDead() requests to
* be stored here. It is used in KisTiledDataManager and
* KisMementoManager.
*
......@@ -119,6 +119,7 @@ private:
void destroy()
{
d->notifyDead();
TileTypeSP::deref(reinterpret_cast<TileTypeSP*>(this), d);
this->MemoryReclaimer::~MemoryReclaimer();
delete this;
......@@ -142,14 +143,18 @@ private:
return ((static_cast<quint32>(row) << 16) | (static_cast<quint32>(col) & 0xFFFF));
}
inline void insert(quint32 key, TileTypeSP value)
inline void insert(quint32 idx, TileTypeSP item)
{
QReadLocker locker(&m_iteratorLock);
TileTypeSP::ref(&value, value.data());
TileType *result = m_map.assign(key, value.data());
TileTypeSP::ref(&item, item.data());
TileType *tile = 0;
if (result) {
m_map.getGC().enqueue(&MemoryReclaimer::destroy, new MemoryReclaimer(result));
{
QReadLocker locker(&m_iteratorLock);
tile = m_map.assign(idx, item.data());
}
if (tile) {
m_map.getGC().enqueue(&MemoryReclaimer::destroy, new MemoryReclaimer(tile));
} else {
m_numTiles.fetchAndAddRelaxed(1);
}
......@@ -157,16 +162,15 @@ private:
m_map.getGC().update(m_map.migrationInProcess());
}
inline bool erase(quint32 key)
inline bool erase(quint32 idx)
{
bool wasDeleted = false;
TileType *result = m_map.erase(key);
TileType *tile = m_map.erase(idx);
if (result) {
if (tile) {
wasDeleted = true;
result->notifyDead();
m_numTiles.fetchAndSubRelaxed(1);
m_map.getGC().enqueue(&MemoryReclaimer::destroy, new MemoryReclaimer(result));
m_map.getGC().enqueue(&MemoryReclaimer::destroy, new MemoryReclaimer(tile));
}
m_map.getGC().update(m_map.migrationInProcess());
......@@ -182,6 +186,7 @@ private:
*/
QReadWriteLock m_defaultPixelDataLock;
mutable QReadWriteLock m_iteratorLock;
std::atomic_flag m_lazyLock = ATOMIC_FLAG_INIT;
QAtomicInt m_numTiles;
KisTileData *m_defaultTileData;
......@@ -232,8 +237,10 @@ public:
{
TileTypeSP tile = m_iter.getValue();
next();
m_ht->deleteTile(tile);
newHashTable->addTile(tile);
quint32 idx = m_ht->calculateHash(tile->col(), tile->row());
m_ht->erase(idx);
newHashTable->insert(idx, tile);
}
private:
......@@ -257,7 +264,7 @@ KisTileHashTableTraits2<T>::KisTileHashTableTraits2(const KisTileHashTableTraits
typename ConcurrentMap<quint32, TileType*>::Iterator iter(ht.m_map);
while (iter.isValid()) {
TileTypeSP tile = TileTypeSP(new TileType(*iter.getValue(), m_mementoManager));
TileTypeSP tile = new TileType(*iter.getValue(), m_mementoManager);
insert(iter.getKey(), tile);
iter.next();
}
......@@ -267,7 +274,6 @@ template <class T>
KisTileHashTableTraits2<T>::~KisTileHashTableTraits2()
{
clear();
m_map.getGC().flush();
setDefaultTileData(0);
}
......@@ -281,38 +287,46 @@ template <class T>
typename KisTileHashTableTraits2<T>::TileTypeSP KisTileHashTableTraits2<T>::getExistingTile(qint32 col, qint32 row)
{
quint32 idx = calculateHash(col, row);
TileTypeSP result = m_map.get(idx);
TileTypeSP tile = m_map.get(idx);
m_map.getGC().update(m_map.migrationInProcess());
return result;
return tile;
}
template <class T>
typename KisTileHashTableTraits2<T>::TileTypeSP KisTileHashTableTraits2<T>::getTileLazy(qint32 col, qint32 row, bool &newTile)
{
QReadLocker locker(&m_iteratorLock);
newTile = false;
TileTypeSP tile;
quint32 idx = calculateHash(col, row);
typename ConcurrentMap<quint32, TileType*>::Mutator mutator = m_map.insertOrFind(idx);
TileTypeSP tile = m_map.get(idx);
if (!mutator.getValue()) {
{
QReadLocker locker(&m_defaultPixelDataLock);
tile = new TileType(col, row, m_defaultTileData, m_mementoManager);
}
TileTypeSP::ref(&tile, tile.data());
TileType *result = mutator.exchangeValue(tile.data());
if (!tile) {
while (m_lazyLock.test_and_set(std::memory_order_acquire));
if (result) {
m_map.getGC().enqueue(&MemoryReclaimer::destroy, new MemoryReclaimer(result));
} else {
newTile = true;
m_numTiles.fetchAndAddRelaxed(1);
if (!(tile = m_map.get(idx))) {
{
QReadLocker locker(&m_defaultPixelDataLock);
tile = new TileType(col, row, m_defaultTileData, m_mementoManager);
}
TileTypeSP::ref(&tile, tile.data());
TileType *item = 0;
{
QReadLocker locker(&m_iteratorLock);
item = m_map.assign(idx, tile.data());
}
if (item) {
m_map.getGC().enqueue(&MemoryReclaimer::destroy, new MemoryReclaimer(item));
} else {
newTile = true;
m_numTiles.fetchAndAddRelaxed(1);
}
tile = m_map.get(idx);
}
tile = m_map.get(idx);
} else {
tile = mutator.getValue();
m_lazyLock.clear(std::memory_order_release);
}
m_map.getGC().update(m_map.migrationInProcess());
......@@ -359,13 +373,17 @@ template<class T>
void KisTileHashTableTraits2<T>::clear()
{
QWriteLocker locker(&m_iteratorLock);
typename ConcurrentMap<quint32, TileType*>::Iterator iter(m_map);
TileType *tile = 0;
while (iter.isValid()) {
tile = m_map.erase(iter.getKey());
tile->notifyDead();
m_map.getGC().enqueue(&MemoryReclaimer::destroy, new MemoryReclaimer(tile));
if (tile) {
m_map.getGC().enqueue(&MemoryReclaimer::destroy, new MemoryReclaimer(tile));
}
iter.next();
}
......@@ -392,7 +410,7 @@ inline void KisTileHashTableTraits2<T>::setDefaultTileData(KisTileData *defaultT
template <class T>
inline KisTileData* KisTileHashTableTraits2<T>::defaultTileData()
{
QReadLocker guard(&m_defaultPixelDataLock);
QReadLocker locker(&m_defaultPixelDataLock);
return m_defaultTileData;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment