koffice/krita/image/tiles
Boudewijn Rempt
boud at valdyas.org
Mon Jun 4 22:50:08 CEST 2007
SVN commit 671476 by rempt:
CCMAIL:kimageshop at kde.org
Given that everything was serialized anyway because of the BKL, remove the other
two mutexes -- this should help a lot with performance, but actually, I think
I noticed a degradation. Please test!
M +20 -66 kis_tilemanager.cc
M +15 -6 kis_tilemanager.h
--- trunk/koffice/krita/image/tiles/kis_tilemanager.cc #671475:671476
@@ -48,7 +48,9 @@
// ### use QMutexLocker for the all mutexes
-KisTileManager::KisTileManager() {
+KisTileManager::KisTileManager()
+ : m_bigKritaLock( QMutex::Recursive )
+{
Q_ASSERT(KisTileManager::m_singleton == 0);
KisTileManager::m_singleton = this;
@@ -79,13 +81,6 @@
}
counter = 0;
-
- // XXX Why pointers to mutexes, and not as member objects?
- m_poolMutex = new QMutex(QMutex::Recursive);
- m_swapMutex = new QMutex(QMutex::Recursive);
- // Catchall until this is code is audited for threading.
- // Actually, why have more than one lock at all here? Much safer with less locks
- m_bigKritaLock = new QMutex(QMutex::Recursive);
}
KisTileManager::~KisTileManager() {
@@ -117,10 +112,6 @@
delete [] m_poolPixelSizes;
delete [] m_pools;
-
- delete m_poolMutex;
- delete m_swapMutex;
- delete m_bigKritaLock;
// Where did this go to? delete [] m_poolFreeList;
}
@@ -135,8 +126,7 @@
void KisTileManager::registerTile(KisTile* tile)
{
- QMutexLocker lock(m_bigKritaLock);
- m_swapMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
TileInfo* info = new TileInfo();
info->tile = tile;
@@ -162,15 +152,13 @@
if (++counter % 50 == 0)
printInfo();
- m_swapMutex->unlock();
}
void KisTileManager::deregisterTile(KisTile* tile) {
- QMutexLocker lock(m_bigKritaLock);
- m_swapMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
+
if (!m_tileMap.contains(tile)) {
- m_swapMutex->unlock();
return;
}
// Q_ASSERT(m_tileMap.contains(tile));
@@ -217,13 +205,11 @@
doSwapping();
- m_swapMutex->unlock();
}
void KisTileManager::ensureTileLoaded(const KisTile* tile)
{
- QMutexLocker lock(m_bigKritaLock);
- m_swapMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
TileInfo* info = m_tileMap[tile];
if (info->validNode) {
@@ -235,13 +221,11 @@
fromSwap(info);
}
- m_swapMutex->unlock();
}
void KisTileManager::maySwapTile(const KisTile* tile)
{
- QMutexLocker lock(m_bigKritaLock);
- m_swapMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
TileInfo* info = m_tileMap[tile];
m_swappableList.push_back(info);
@@ -250,16 +234,13 @@
doSwapping();
- m_swapMutex->unlock();
}
void KisTileManager::fromSwap(TileInfo* info)
{
- QMutexLocker lock(m_bigKritaLock);
- m_swapMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
if (info->inMem) {
- m_swapMutex->unlock();
return;
}
@@ -272,7 +253,6 @@
if (!kritaMmap(info->tile->m_data, 0, info->size, PROT_READ | PROT_WRITE, MAP_SHARED,
info->file->handle(), info->filePos)) {
kWarning() << "fromSwap failed!" << endl;
- m_swapMutex->unlock();
return;
}
@@ -282,16 +262,13 @@
m_currentInMem++;
m_bytesInMem += info->size;
- m_swapMutex->unlock();
}
void KisTileManager::toSwap(TileInfo* info) {
- QMutexLocker lock(m_bigKritaLock);
- m_swapMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
//Q_ASSERT(info->inMem);
if (!info || !info->inMem) {
- m_swapMutex->unlock();
return;
}
@@ -357,7 +334,6 @@
printInfo();
m_swapForbidden = true;
- m_swapMutex->unlock();
return;
}
@@ -372,7 +348,6 @@
if(!file) {
kWarning() << "Opening the file as QFile failed" << endl;
m_swapForbidden = true;
- m_swapMutex->unlock();
return;
}
@@ -382,19 +357,16 @@
fd, info->filePos)) {
kWarning() << "Initial mmap failed" << endl;
m_swapForbidden = true;
- m_swapMutex->unlock();
return;
}
memcpy(data, info->tile->m_data, info->size);
munmap(data, info->size);
- m_poolMutex->lock();
if (isPoolTile(tile->m_data, tile->m_pixelSize))
reclaimTileToPool(tile->m_data, tile->m_pixelSize);
else
delete[] tile->m_data;
- m_poolMutex->unlock();
tile->m_data = 0;
} else {
@@ -413,16 +385,13 @@
m_currentInMem--;
m_bytesInMem -= info->size;
- m_swapMutex->unlock();
}
void KisTileManager::doSwapping()
{
- QMutexLocker lock(m_bigKritaLock);
- m_swapMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
if (m_swapForbidden || m_currentInMem <= m_maxInMem) {
- m_swapMutex->unlock();
return;
}
@@ -438,12 +407,11 @@
#endif
- m_swapMutex->unlock();
}
void KisTileManager::printInfo()
{
- QMutexLocker lock(m_bigKritaLock);
+ QMutexLocker lock(&m_bigKritaLock);
kDebug(DBG_AREA_TILES) << m_bytesInMem << " out of " << m_bytesTotal << " bytes in memory\n";
kDebug(DBG_AREA_TILES) << m_currentInMem << " out of " << m_tileMap.size() << " tiles in memory\n";
kDebug(DBG_AREA_TILES) << m_files.size() << " swap files in use" << endl;
@@ -469,40 +437,34 @@
quint8* KisTileManager::requestTileData(qint32 pixelSize)
{
- QMutexLocker lock(m_bigKritaLock);
- m_swapMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
quint8* data = findTileFor(pixelSize);
if ( data ) {
- m_swapMutex->unlock();
return data;
}
- m_swapMutex->unlock();
+
return new quint8[m_tileSize * pixelSize];
}
void KisTileManager::dontNeedTileData(quint8* data, qint32 pixelSize)
{
- QMutexLocker lock(m_bigKritaLock);
- m_poolMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
if (isPoolTile(data, pixelSize)) {
reclaimTileToPool(data, pixelSize);
} else
delete[] data;
- m_poolMutex->unlock();
}
quint8* KisTileManager::findTileFor(qint32 pixelSize)
{
- QMutexLocker lock(m_bigKritaLock);
- m_poolMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
for (int i = 0; i < 4; i++) {
if (m_poolPixelSizes[i] == pixelSize) {
if (!m_poolFreeList[i].isEmpty()) {
quint8* data = m_poolFreeList[i].front();
m_poolFreeList[i].pop_front();
- m_poolMutex->unlock();
return data;
}
}
@@ -513,56 +475,48 @@
// j = 1 because we return the first element, so no need to add it to the freelist
for (int j = 1; j < m_tilesPerPool; j++)
m_poolFreeList[i].append(&m_pools[i][j * pixelSize * m_tileSize]);
- m_poolMutex->unlock();
return m_pools[i];
}
}
- m_poolMutex->unlock();
return 0;
}
bool KisTileManager::isPoolTile(quint8* data, qint32 pixelSize) {
- QMutexLocker lock(m_bigKritaLock);
+ QMutexLocker lock(&m_bigKritaLock);
if (data == 0)
return false;
- m_poolMutex->lock();
for (int i = 0; i < 4; i++) {
if (m_poolPixelSizes[i] == pixelSize) {
bool b = data >= m_pools[i]
&& data < m_pools[i] + pixelSize * m_tileSize * m_tilesPerPool;
if (b) {
- m_poolMutex->unlock();
return true;
}
}
}
- m_poolMutex->unlock();
return false;
}
void KisTileManager::reclaimTileToPool(quint8* data, qint32 pixelSize) {
- QMutexLocker lock(m_bigKritaLock);
- m_poolMutex->lock();
+ QMutexLocker lock(&m_bigKritaLock);
+
for (int i = 0; i < 4; i++) {
if (m_poolPixelSizes[i] == pixelSize)
if (data >= m_pools[i] && data < m_pools[i] + pixelSize * m_tileSize * m_tilesPerPool) {
m_poolFreeList[i].append(data);
}
}
- m_poolMutex->unlock();
}
void KisTileManager::configChanged() {
- QMutexLocker lock(m_bigKritaLock);
+ QMutexLocker lock(&m_bigKritaLock);
KConfigGroup cfg = KGlobal::config()->group("");
m_maxInMem = cfg.readEntry("maxtilesinmem", 4000);
m_swappiness = cfg.readEntry("swappiness", 100);
- m_swapMutex->lock();
doSwapping();
- m_swapMutex->unlock();
}
bool KisTileManager::kritaMmap(quint8*& result, void *start, size_t length,
--- trunk/koffice/krita/image/tiles/kis_tilemanager.h #671475:671476
@@ -43,7 +43,7 @@
* * tries to preallocate and recycle some tiles to make future allocations faster
* (not done yet)
*
- * XXX: Use QReadWriteLock, QReadLocker and QWriteLocker to make sure everyone can
+ * XXX: Use QReadWriteLock, QReadLocker and QWriteLocker to make sure everyone can
* read any tile, as long as nobody is writing to it. (bsar)
* See: http://doc.trolltech.com/qq/qq14-threading.html
*/
@@ -89,9 +89,19 @@
// filePos is the position inside the file; size is the actual size, fsize is the size
// being used in the swap for this tile (may be larger!)
// The file points to 0 if it is not swapped, and to the relevant TempFile otherwise
- struct TileInfo { KisTile *tile; KTemporaryFile* file; off_t filePos; int size; int fsize;
+ struct TileInfo {
+ KisTile *tile;
+ KTemporaryFile* file;
+ off_t filePos;
+ int size;
+ int fsize;
Q3ValueList<TileInfo*>::iterator node;
- bool inMem; bool onFile; bool mmapped; bool validNode; };
+ bool inMem;
+ bool onFile;
+ bool mmapped;
+ bool validNode;
+ };
+
typedef struct { KTemporaryFile* file; off_t filePos; int size; } FreeInfo;
typedef QHash<const KisTile*, TileInfo*> TileMap;
typedef Q3ValueList<TileInfo*> TileList;
@@ -116,10 +126,9 @@
qint32 *m_poolPixelSizes;
qint32 m_tilesPerPool;
PoolFreeList *m_poolFreeList;
- QMutex * m_poolMutex;
- QMutex * m_swapMutex;
- QMutex * m_bigKritaLock; // The 'BKL' ;)
+ QMutex m_bigKritaLock; // The 'BKL' ;)
+
// This is the constant that we will use to see if we want to add a new tempfile
// We use 1<<30 (one gigabyte) because apparently 32bit systems don't really like very
// large files.
More information about the kimageshop
mailing list