From 89a25a3ef169cb42c54ef1ddcb28f70b24f923ab Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Sun, 9 Jun 2019 20:01:36 +0200 Subject: Don't count all engines as "in use" in cache Back in 36cb3f3f655a9090c82de609010cbfb88651a0f3, we started properly reference counting font engines as they were entered into the font cache. Prior to this, ref.load == 0 would mean that the engine was essentially owned by the cache. When the change was made, the condition that an engine must be in use if its reference is != 0 remained, and the result of this was used to calculate the limit for when the cache should be flushed. Since this limit was miscalculated, the cache would keep growing, even if it only contained unused font engines. [ChangeLog][QtGui][Text] Fixed a bug which could cause the font cache to grow larger than it was supposed to. Task-number: QTBUG-76219 Change-Id: I4d1541756f3bdf5bd9b0301bf47c6db2e220716a Reviewed-by: Konstantin Ritt --- src/gui/text/qfont.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/gui/text/qfont.cpp') diff --git a/src/gui/text/qfont.cpp b/src/gui/text/qfont.cpp index 69255fd59d..fe4fa4929a 100644 --- a/src/gui/text/qfont.cpp +++ b/src/gui/text/qfont.cpp @@ -2970,7 +2970,7 @@ void QFontCache::decreaseCache() it.value().data->ref.load(), engineCacheCount.value(it.value().data), it.value().data->cache_cost); - if (it.value().data->ref.load() != 0) + if (it.value().data->ref.load() > engineCacheCount.value(it.value().data)) in_use_cost += it.value().data->cache_cost / engineCacheCount.value(it.value().data); } -- cgit v1.2.3 From 34fe9232dbf6a9fe58ebc4c7680bb67d2f642c40 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Mon, 10 Jun 2019 11:08:29 +0200 Subject: Port from QAtomic::load() to loadRelaxed() Semi-automated, just needed ~20 manual fixes: $ find \( -iname \*.cpp -or -iname \*.h \) -exec perl -pe 's/(\.|->)load\(\)/$1loadRelaxed\(\)/g' -i \{\} + $ find \( -iname \*.cpp -or -iname \*.h \) -exec perl -pe 's/(\.|->)store\(/$1storeRelaxed\(/g' -i \{\} + It can be easily improved (e.g. for store check that there are no commas after the opening parens). The most common offender is QLibrary::load, and some code using std::atomic directly. Change-Id: I07c38a3c8ed32c924ef4999e85c7e45cf48f0f6c Reviewed-by: Marc Mutz --- src/gui/text/qfont.cpp | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'src/gui/text/qfont.cpp') diff --git a/src/gui/text/qfont.cpp b/src/gui/text/qfont.cpp index 0249a20cc6..5555422b82 100644 --- a/src/gui/text/qfont.cpp +++ b/src/gui/text/qfont.cpp @@ -336,7 +336,7 @@ QFontEngineData::QFontEngineData() QFontEngineData::~QFontEngineData() { - Q_ASSERT(ref.load() == 0); + Q_ASSERT(ref.loadRelaxed() == 0); for (int i = 0; i < QChar::ScriptCount; ++i) { if (engines[i]) { if (!engines[i]->ref.deref()) @@ -604,7 +604,7 @@ QFont::QFont(QFontPrivate *data) */ void QFont::detach() { - if (d->ref.load() == 1) { + if (d->ref.loadRelaxed() == 1) { if (d->engineData && !d->engineData->ref.deref()) delete d->engineData; d->engineData = 0; @@ -625,7 +625,7 @@ void QFont::detach() */ void QFontPrivate::detachButKeepEngineData(QFont *font) { - if (font->d->ref.load() == 1) + if (font->d->ref.loadRelaxed() == 1) return; QFontEngineData *engineData = font->d->engineData; @@ -2833,7 +2833,7 @@ void QFontCache::clear() delete data; } else { FC_DEBUG("QFontCache::clear: engineData %p still has refcount %d", - data, data->ref.load()); + data, data->ref.loadRelaxed()); } ++it; } @@ -2857,7 +2857,7 @@ void QFontCache::clear() delete engine; } else if (cacheCount == 0) { FC_DEBUG("QFontCache::clear: engine %p still has refcount %d", - engine, engine->ref.load()); + engine, engine->ref.loadRelaxed()); } it.value().data = 0; } @@ -2927,7 +2927,7 @@ void QFontCache::updateHitCountAndTimeStamp(Engine &value) FC_DEBUG("QFontCache: found font engine\n" " %p: timestamp %4u hits %3u ref %2d/%2d, type %d", value.data, value.timestamp, value.hits, - value.data->ref.load(), engineCacheCount.value(value.data), + value.data->ref.loadRelaxed(), engineCacheCount.value(value.data), value.data->type()); } @@ -2937,7 +2937,7 @@ void QFontCache::insertEngine(const Key &key, QFontEngine *engine, bool insertMu Q_ASSERT(key.multi == (engine->type() == QFontEngine::Multi)); #ifdef QFONTCACHE_DEBUG - FC_DEBUG("QFontCache: inserting new engine %p, refcount %d", engine, engine->ref.load()); + FC_DEBUG("QFontCache: inserting new engine %p, refcount %d", engine, engine->ref.loadRelaxed()); if (!insertMulti && engineCache.contains(key)) { FC_DEBUG(" QFontCache already contains engine %p for key=(%g %g %d %d %d)", engineCache.value(key).data, key.def.pointSize, @@ -3026,9 +3026,9 @@ void QFontCache::decreaseCache() EngineDataCache::ConstIterator it = engineDataCache.constBegin(), end = engineDataCache.constEnd(); for (; it != end; ++it) { - FC_DEBUG(" %p: ref %2d", it.value(), int(it.value()->ref.load())); + FC_DEBUG(" %p: ref %2d", it.value(), int(it.value()->ref.loadRelaxed())); - if (it.value()->ref.load() != 1) + if (it.value()->ref.loadRelaxed() != 1) in_use_cost += engine_data_cost; } } @@ -3041,10 +3041,10 @@ void QFontCache::decreaseCache() for (; it != end; ++it) { FC_DEBUG(" %p: timestamp %4u hits %2u ref %2d/%2d, cost %u bytes", it.value().data, it.value().timestamp, it.value().hits, - it.value().data->ref.load(), engineCacheCount.value(it.value().data), + it.value().data->ref.loadRelaxed(), engineCacheCount.value(it.value().data), it.value().data->cache_cost); - if (it.value().data->ref.load() > engineCacheCount.value(it.value().data)) + if (it.value().data->ref.loadRelaxed() > engineCacheCount.value(it.value().data)) in_use_cost += it.value().data->cache_cost / engineCacheCount.value(it.value().data); } @@ -3093,7 +3093,7 @@ void QFontCache::decreaseCache() // clean out all unused engine data EngineDataCache::Iterator it = engineDataCache.begin(); while (it != engineDataCache.end()) { - if (it.value()->ref.load() == 1) { + if (it.value()->ref.loadRelaxed() == 1) { FC_DEBUG(" %p", it.value()); decreaseCost(sizeof(QFontEngineData)); it.value()->ref.deref(); @@ -3121,7 +3121,7 @@ void QFontCache::decreaseCache() EngineCache::Iterator jt = end; for ( ; it != end; ++it) { - if (it.value().data->ref.load() != engineCacheCount.value(it.value().data)) + if (it.value().data->ref.loadRelaxed() != engineCacheCount.value(it.value().data)) continue; if (it.value().timestamp < oldest && it.value().hits <= least_popular) { @@ -3135,7 +3135,7 @@ void QFontCache::decreaseCache() if (it != end) { FC_DEBUG(" %p: timestamp %4u hits %2u ref %2d/%2d, type %d", it.value().data, it.value().timestamp, it.value().hits, - it.value().data->ref.load(), engineCacheCount.value(it.value().data), + it.value().data->ref.loadRelaxed(), engineCacheCount.value(it.value().data), it.value().data->type()); QFontEngine *fontEngine = it.value().data; @@ -3150,7 +3150,7 @@ void QFontCache::decreaseCache() } } // and delete the last occurrence - Q_ASSERT(fontEngine->ref.load() == 0); + Q_ASSERT(fontEngine->ref.loadRelaxed() == 0); decreaseCost(fontEngine->cache_cost); delete fontEngine; engineCacheCount.remove(fontEngine); -- cgit v1.2.3