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 From 8da3eea4fb702c2dc369c1628e91a034569aa9f0 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 25 Jun 2019 22:21:44 +0200 Subject: Optimize some atomic counters Define the static QAtomic at file scope to avoid GCC's pessimisation with function-static QAtomic (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79561), and make sure the initial value is 0, so it ends up in BSS, not TEXT. In QRhi..., don't create a static instance of the wrapper class, use a file- static atomic, too. This turns the class into a glorified namespace. Change-Id: I707f628e2b434330028077223071716d5704ba32 Reviewed-by: Thiago Macieira --- src/gui/text/qfont.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/gui/text/qfont.cpp') diff --git a/src/gui/text/qfont.cpp b/src/gui/text/qfont.cpp index 5555422b82..2a1d207702 100644 --- a/src/gui/text/qfont.cpp +++ b/src/gui/text/qfont.cpp @@ -2799,12 +2799,12 @@ void QFontCache::cleanup() cache->setLocalData(0); } -QBasicAtomicInt font_cache_id = Q_BASIC_ATOMIC_INITIALIZER(1); +static QBasicAtomicInt font_cache_id = Q_BASIC_ATOMIC_INITIALIZER(0); QFontCache::QFontCache() : QObject(), total_cost(0), max_cost(min_cost), current_timestamp(0), fast(false), timer_id(-1), - m_id(font_cache_id.fetchAndAddRelaxed(1)) + m_id(font_cache_id.fetchAndAddRelaxed(1) + 1) { } -- cgit v1.2.3 From aadf64f08486766bb6d40bcaae47e9b4d6411c28 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 27 Jun 2019 19:01:31 +0200 Subject: QtGui: port from QMutex::Recursive to QRecursiveMutex Change-Id: I1ce4fcfa1bfb9a89fe3ebe61d049b9b3100fd700 Reviewed-by: Thiago Macieira --- 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 2a1d207702..97e73f0723 100644 --- a/src/gui/text/qfont.cpp +++ b/src/gui/text/qfont.cpp @@ -208,7 +208,7 @@ QFontPrivate::~QFontPrivate() scFont = 0; } -extern QMutex *qt_fontdatabase_mutex(); +extern QRecursiveMutex *qt_fontdatabase_mutex(); #define QT_FONT_ENGINE_FROM_DATA(data, script) data->engines[script] -- cgit v1.2.3