From 1b60bc8d3dc8a6d4b06206f59865e6ecd3ea3c28 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Mon, 10 Mar 2014 16:02:57 +0100 Subject: [PATCH 1/7] Fix label for render thread. Change-Id: I8162d2dd025853055fff3d258e62eba5889ef3a8 Reviewed-by: Kai Koehne --- plugins/qmlprofilerextension/scenegraphtimelinemodel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/qmlprofilerextension/scenegraphtimelinemodel.cpp b/plugins/qmlprofilerextension/scenegraphtimelinemodel.cpp index 26fc5ed7c19..574f115e245 100644 --- a/plugins/qmlprofilerextension/scenegraphtimelinemodel.cpp +++ b/plugins/qmlprofilerextension/scenegraphtimelinemodel.cpp @@ -110,7 +110,7 @@ QString labelForSGType(int t) { switch ((SceneGraphCategoryType)t) { case SceneGraphRenderThread: - return QCoreApplication::translate("SceneGraphTimelineModel", "Renderer Thread"); + return QCoreApplication::translate("SceneGraphTimelineModel", "Render Thread"); case SceneGraphGUIThread: return QCoreApplication::translate("SceneGraphTimelineModel", "GUI Thread"); default: return QString(); From 03c324130677f3d1be1ad57d49d07dd6b61ff15c Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Wed, 26 Feb 2014 17:44:58 +0100 Subject: [PATCH 2/7] Saturate colors some more and use color selection from base class The original colors were hard to discern from the background on certain devices. Change-Id: I276f2cd76e0a1be40040bf5a0557a288d10d37d9 Reviewed-by: Erik Verbruggen --- plugins/qmlprofilerextension/pixmapcachemodel.cpp | 5 ++--- plugins/qmlprofilerextension/pixmapcachemodel.h | 2 ++ plugins/qmlprofilerextension/scenegraphtimelinemodel.cpp | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/qmlprofilerextension/pixmapcachemodel.cpp b/plugins/qmlprofilerextension/pixmapcachemodel.cpp index def302e4755..dc26db9c772 100644 --- a/plugins/qmlprofilerextension/pixmapcachemodel.cpp +++ b/plugins/qmlprofilerextension/pixmapcachemodel.cpp @@ -90,10 +90,9 @@ QColor PixmapCacheModel::getColor(int index) const { Q_D(const PixmapCacheModel); if (d->range(index).pixmapEventType == PixmapCacheCountChanged) - return QColor::fromHsl(240, 76, 166); + return getColorByHue(PixmapCacheCountHue); - int ndx = getEventId(index); - return QColor::fromHsl((ndx*25)%360, 76, 166); + return getEventColor(index); } float PixmapCacheModel::getHeight(int index) const diff --git a/plugins/qmlprofilerextension/pixmapcachemodel.h b/plugins/qmlprofilerextension/pixmapcachemodel.h index e1bafd65527..b0470f37130 100644 --- a/plugins/qmlprofilerextension/pixmapcachemodel.h +++ b/plugins/qmlprofilerextension/pixmapcachemodel.h @@ -71,6 +71,8 @@ public: void clear(); private: + static const int PixmapCacheCountHue = 240; + class PixmapCacheModelPrivate; Q_DECLARE_PRIVATE(PixmapCacheModel) }; diff --git a/plugins/qmlprofilerextension/scenegraphtimelinemodel.cpp b/plugins/qmlprofilerextension/scenegraphtimelinemodel.cpp index 574f115e245..584186c0ac2 100644 --- a/plugins/qmlprofilerextension/scenegraphtimelinemodel.cpp +++ b/plugins/qmlprofilerextension/scenegraphtimelinemodel.cpp @@ -103,7 +103,7 @@ QColor SceneGraphTimelineModel::getColor(int index) const double fpsFraction = 1 / (eventDuration * 60.0); if (fpsFraction > 1.0) fpsFraction = 1.0; - return QColor::fromHsl((fpsFraction*96)+10, 76, 166); + return getFractionColor(fpsFraction); } QString labelForSGType(int t) From fbfde55dc23d1bc8c6ea9a563a5a478e2bccf72f Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Fri, 21 Mar 2014 13:28:54 +0100 Subject: [PATCH 3/7] Make the cache count row lie less. The height of the events there didn't correspond to the actual cache fill state. Change-Id: I8d0415d4ec4e9e97fb1821bb8c0f5b536e21abd4 Reviewed-by: Kai Koehne --- plugins/qmlprofilerextension/pixmapcachemodel.cpp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/plugins/qmlprofilerextension/pixmapcachemodel.cpp b/plugins/qmlprofilerextension/pixmapcachemodel.cpp index dc26db9c772..1ff6e6ab99a 100644 --- a/plugins/qmlprofilerextension/pixmapcachemodel.cpp +++ b/plugins/qmlprofilerextension/pixmapcachemodel.cpp @@ -98,17 +98,10 @@ QColor PixmapCacheModel::getColor(int index) const float PixmapCacheModel::getHeight(int index) const { Q_D(const PixmapCacheModel); - if (d->range(index).pixmapEventType == PixmapCacheCountChanged) { - float scale = d->maxCacheSize - d->minCacheSize; - float fraction = 1.0f; - if (scale > 1) - fraction = (float)(d->range(index).cacheSize - - d->minCacheSize) / scale; - - return fraction * 0.85f + 0.15f; - } - - return 1.0f; + if (d->range(index).pixmapEventType == PixmapCacheCountChanged) + return 0.15 + (float)d->range(index).cacheSize * 0.85 / (float)d->maxCacheSize; + else + return 1.0f; } QString getFilenameOnly(QString absUrl) From 4540e917736ba8f7fa187dd6c93ef62e91b37d01 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Fri, 21 Mar 2014 16:52:28 +0100 Subject: [PATCH 4/7] Fix pixmap cache profiling for multiple pixmaps with equal URLs Apply a heuristic to figure out which pixmaps are causing which events. As the only means of identification we can get from the application is the URL this will never be perfect. However, using some knowledge about the inner workings of pixmap loading we can get usable results. Change-Id: I42d7af7499ed7692ed87e68e8fd12c7528e08b3e Reviewed-by: Kai Koehne --- .../qmlprofilerextension/pixmapcachemodel.cpp | 318 +++++++++++++++--- .../qmlprofilerextension/pixmapcachemodel.h | 1 + 2 files changed, 263 insertions(+), 56 deletions(-) diff --git a/plugins/qmlprofilerextension/pixmapcachemodel.cpp b/plugins/qmlprofilerextension/pixmapcachemodel.cpp index 1ff6e6ab99a..f5d08083e83 100644 --- a/plugins/qmlprofilerextension/pixmapcachemodel.cpp +++ b/plugins/qmlprofilerextension/pixmapcachemodel.cpp @@ -23,12 +23,45 @@ #include "qmlprofiler/singlecategorytimelinemodel_p.h" #include +#include namespace QmlProfilerExtension { namespace Internal { using namespace QmlProfiler; +enum CacheState { + Uncached, // After loading started (or some other proof of existence) or after uncaching + ToBeCached, // After determining the pixmap is to be cached but before knowing its size + Cached, // After caching a pixmap or determining the size of a ToBeCached pixmap + Uncacheable, // If loading failed without ToBeCached or after a corrupt pixmap has been uncached + Corrupt // If after ToBeCached we learn that loading failed +}; + +enum LoadState { + Initial, + Loading, + Finished, + Error +}; + +struct PixmapState { + PixmapState(int width, int height, CacheState cache = Uncached) : + size(width, height), started(-1), loadState(Initial), cacheState(cache) {} + PixmapState(CacheState cache = Uncached) : started(-1), loadState(Initial), cacheState(cache) {} + QSize size; + int started; + LoadState loadState; + CacheState cacheState; +}; + +struct Pixmap { + Pixmap() {} + Pixmap(const QString &url) : url(url), sizes(1) {} + QString url; + QVector sizes; +}; + class PixmapCacheModel::PixmapCacheModelPrivate : public SortedTimelineModel @@ -38,9 +71,10 @@ public: void resizeUnfinishedLoads(); void flattenLoads(); void computeRowCounts(); + int updateCacheCount(int lastCacheSizeEvent, qint64 startTime, qint64 pixSize, + PixmapCacheEvent &newEvent); - QVector < QString > pixmapUrls; - QVector < QPair > pixmapSizes; + QVector pixmaps; int expandedRowCount; int collapsedRowCount; void addVP(QVariantList &l, QString label, qint64 time) const; @@ -129,11 +163,13 @@ const QVariantList PixmapCacheModel::getLabelsForCategory(int category) const result << element; } - for (int i=0; i < d->pixmapUrls.count(); i++) { + for (int i=0; i < d->pixmaps.count(); i++) { // Loading QVariantMap element; - element.insert(QLatin1String("displayName"), QVariant(getFilenameOnly(d->pixmapUrls[i]))); - element.insert(QLatin1String("description"), QVariant(getFilenameOnly(d->pixmapUrls[i]))); + element.insert(QLatin1String("displayName"), + QVariant(getFilenameOnly(d->pixmaps[i].url))); + element.insert(QLatin1String("description"), + QVariant(getFilenameOnly(d->pixmaps[i].url))); element.insert(QLatin1String("id"), QVariant(i+1)); result << element; @@ -173,20 +209,23 @@ const QVariantList PixmapCacheModel::getEventDetails(int index) const { QVariantMap res; - res.insert(tr("File"), QVariant(getFilenameOnly(d->pixmapUrls[ev->urlIndex]))); + res.insert(tr("File"), QVariant(getFilenameOnly(d->pixmaps[ev->urlIndex].url))); result << res; } { QVariantMap res; - res.insert(tr("Width"), QVariant(QString::fromLatin1("%1 px").arg(d->pixmapSizes[ev->urlIndex].first))); + res.insert(tr("Width"), QVariant(QString::fromLatin1("%1 px") + .arg(d->pixmaps[ev->urlIndex].sizes[ev->sizeIndex].size.width()))); result << res; res.clear(); - res.insert(tr("Height"), QVariant(QString::fromLatin1("%1 px").arg(d->pixmapSizes[ev->urlIndex].second))); + res.insert(tr("Height"), QVariant(QString::fromLatin1("%1 px") + .arg(d->pixmaps[ev->urlIndex].sizes[ev->sizeIndex].size.height()))); result << res; } - if (ev->pixmapEventType == PixmapLoadingStarted && ev->cacheSize == -1) { + if (ev->pixmapEventType == PixmapLoadingStarted && + d->pixmaps[ev->urlIndex].sizes[ev->sizeIndex].loadState != Finished) { QVariantMap res; res.insert(tr("Result"), QVariant(QLatin1String("Load Error"))); result << res; @@ -195,6 +234,36 @@ const QVariantList PixmapCacheModel::getEventDetails(int index) const return result; } +/* Ultimately there is no way to know which cache entry a given event refers to as long as we only + * receive the pixmap URL from the application. Multiple copies of different sizes may be cached + * for each URL. However, we can apply some heuristics to make the result somewhat plausible by + * using the following assumptions: + * + * - PixmapSizeKnown will happen at most once for every cache entry. + * - PixmapSizeKnown cannot happen for entries with PixmapLoadingError and vice versa. + * - PixmapCacheCountChanged can happen for entries with PixmapLoadingError but doesn't have to. + * - Decreasing PixmapCacheCountChanged events can only happen for entries that have seen an + * increasing PixmapCacheCountChanged (but that may have happened before the trace). + * - PixmapCacheCountChanged can happen before or after PixmapSizeKnown. + * - For every PixmapLoadingFinished or PixmapLoadingError there is exactly one + * PixmapLoadingStarted event, but it may be before the trace. + * - For every PixmapLoadingStarted there is exactly one PixmapLoadingFinished or + * PixmapLoadingError, but it may be after the trace. + * - Decreasing PixmapCacheCountChanged events in the presence of corrupt cache entries are more + * likely to clear those entries than other, correctly loaded ones. + * - Increasing PixmapCacheCountChanged events are more likely to refer to correctly loaded entries + * than to ones with PixmapLoadingError. + * - PixmapLoadingFinished and PixmapLoadingError are more likely to refer to cache entries that + * have seen a PixmapLoadingStarted than to ones that haven't. + * + * For each URL we keep an ordered list of pixmaps possibly being loaded and assign new events to + * the first entry that "fits". If multiple sizes of the same pixmap are being loaded concurrently + * we generally assume that the PixmapLoadingFinished and PixmapLoadingError events occur in the + * order we learn about the existence of these sizes, subject to the above constraints. This is not + * necessarily the order the pixmaps are really loaded but it's the best we can do with the given + * information. If they're loaded sequentially the representation is correct. + */ + void PixmapCacheModel::loadData() { Q_D(PixmapCacheModel); @@ -205,8 +274,6 @@ void PixmapCacheModel::loadData() int lastCacheSizeEvent = -1; int cumulatedCount = 0; - QVector < int > pixmapStartPoints; - QVector < int > pixmapCachePoints; foreach (const QmlProfilerDataModel::QmlEventData &event, simpleModel->getEvents()) { if (!eventAccepted(event)) @@ -216,69 +283,192 @@ void PixmapCacheModel::loadData() newEvent.pixmapEventType = event.bindingType; qint64 startTime = event.startTime; - bool isNewEntry = false; - newEvent.urlIndex = d->pixmapUrls.indexOf(event.location.filename); + newEvent.urlIndex = -1; + for (QVector::const_iterator it(d->pixmaps.cend()); it != d->pixmaps.cbegin();) { + if ((--it)->url == event.location.filename) { + newEvent.urlIndex = it - d->pixmaps.cbegin(); + break; + } + } + + newEvent.sizeIndex = -1; if (newEvent.urlIndex == -1) { - isNewEntry = true; - newEvent.urlIndex = d->pixmapUrls.count(); - d->pixmapUrls << event.location.filename; - d->pixmapSizes << QPair(0,0); // default value - pixmapStartPoints << -1; // dummy value to be filled by load event - pixmapCachePoints << -1; // dummy value to be filled by cache event + newEvent.urlIndex = d->pixmaps.count(); + d->pixmaps << Pixmap(event.location.filename); } newEvent.eventId = newEvent.urlIndex + 1; newEvent.rowNumberExpanded = newEvent.urlIndex + 2; + Pixmap &pixmap = d->pixmaps[newEvent.urlIndex]; switch (newEvent.pixmapEventType) { - case PixmapSizeKnown: // pixmap size - d->pixmapSizes[newEvent.urlIndex] = QPair((int)event.numericData1, (int)event.numericData2); - if (pixmapCachePoints[newEvent.urlIndex] == -1) + case PixmapSizeKnown: {// pixmap size + // Look for pixmaps for which we don't know the size, yet and which have actually been + // loaded. + for (QVector::iterator i(pixmap.sizes.begin()); + i != pixmap.sizes.end(); ++i) { + if (i->size.isValid() || i->cacheState == Uncacheable || i->cacheState == Corrupt) + continue; + + // We can't have cached it before we knew the size + Q_ASSERT(i->cacheState != Cached); + + i->size.setWidth(event.numericData1); + i->size.setHeight(event.numericData2); + newEvent.sizeIndex = i - pixmap.sizes.begin(); break; - // else fall through and update cache size - newEvent.pixmapEventType = PixmapCacheCountChanged; + } + + if (newEvent.sizeIndex == -1) { + newEvent.sizeIndex = pixmap.sizes.length(); + pixmap.sizes << PixmapState(event.numericData1, event.numericData2); + } + + PixmapState &state = pixmap.sizes[newEvent.sizeIndex]; + if (state.cacheState == ToBeCached) { + lastCacheSizeEvent = d->updateCacheCount(lastCacheSizeEvent, startTime, + state.size.width() * state.size.height(), newEvent); + state.cacheState = Cached; + } + break; + } case PixmapCacheCountChanged: {// Cache Size Changed Event startTime = event.startTime + 1; // delay 1 ns for proper sorting - newEvent.eventId = 0; - newEvent.rowNumberExpanded = 1; - newEvent.rowNumberCollapsed = 1; - qint64 pixSize = d->pixmapSizes[newEvent.urlIndex].first * d->pixmapSizes[newEvent.urlIndex].second; - qint64 prevSize = 0; - if (lastCacheSizeEvent != -1) { - prevSize = d->range(lastCacheSizeEvent).cacheSize; - if (pixmapCachePoints[newEvent.urlIndex] == -1) { - // else it's a synthesized update and doesn't have a valid cache count - if (event.numericData3 < cumulatedCount) - pixSize = -pixSize; - cumulatedCount = event.numericData3; + bool uncache = cumulatedCount > event.numericData3; + cumulatedCount = event.numericData3; + qint64 pixSize = 0; + + // First try to find a preferred pixmap, which either is Corrupt and will be uncached + // or is uncached and will be cached. + for (QVector::iterator i(pixmap.sizes.begin()); + i != pixmap.sizes.end(); ++i) { + if (uncache && i->cacheState == Corrupt) { + newEvent.sizeIndex = i - pixmap.sizes.begin(); + i->cacheState = Uncacheable; + break; + } else if (!uncache && i->cacheState == Uncached) { + newEvent.sizeIndex = i - pixmap.sizes.begin(); + if (i->size.isValid()) { + pixSize = i->size.width() * i->size.height(); + i->cacheState = Cached; + } else { + i->cacheState = ToBeCached; + } + break; } - d->insertEnd(lastCacheSizeEvent, startTime - d->range(lastCacheSizeEvent).start); } - newEvent.cacheSize = prevSize + pixSize; - lastCacheSizeEvent = d->insertStart(startTime, newEvent); - pixmapCachePoints[newEvent.urlIndex] = lastCacheSizeEvent; + + // If none found, check for cached or ToBeCached pixmaps that shall be uncached or + // Error pixmaps that become corrupt cache entries. We also accept Initial to be + // uncached as we may have missed the matching PixmapCacheCountChanged that cached it. + if (newEvent.sizeIndex == -1) { + for (QVector::iterator i(pixmap.sizes.begin()); + i != pixmap.sizes.end(); ++i) { + if (uncache && (i->cacheState == Cached || i->cacheState == ToBeCached || + i->cacheState == Uncached)) { + newEvent.sizeIndex = i - pixmap.sizes.begin(); + if (i->size.isValid()) + pixSize = -i->size.width() * i->size.height(); + i->cacheState = Uncached; + break; + } else if (!uncache && i->cacheState == Uncacheable) { + newEvent.sizeIndex = i - pixmap.sizes.begin(); + i->cacheState = Corrupt; + break; + } + } + } + + // If that does't work, create a new entry. + if (newEvent.sizeIndex == -1) { + newEvent.sizeIndex = pixmap.sizes.length(); + pixmap.sizes << PixmapState(uncache ? Uncached : ToBeCached); + } + + lastCacheSizeEvent = d->updateCacheCount(lastCacheSizeEvent, startTime, pixSize, + newEvent); break; } case PixmapLoadingStarted: // Load - pixmapStartPoints[newEvent.urlIndex] = d->insertStart(startTime, newEvent); + // Look for a pixmap that hasn't been started, yet. There may have been a refcount + // event, which we ignore. + for (QVector::const_iterator i(pixmap.sizes.cbegin()); + i != pixmap.sizes.cend(); ++i) { + if (i->loadState == Initial) { + newEvent.sizeIndex = i - pixmap.sizes.cbegin(); + break; + } + } + if (newEvent.sizeIndex == -1) { + newEvent.sizeIndex = pixmap.sizes.length(); + pixmap.sizes << PixmapState(); + } + pixmap.sizes[newEvent.sizeIndex].started = d->insertStart(startTime, newEvent); + pixmap.sizes[newEvent.sizeIndex].loadState = Loading; break; case PixmapLoadingFinished: case PixmapLoadingError: { - int loadIndex = pixmapStartPoints[newEvent.urlIndex]; - if (!isNewEntry && loadIndex != -1) { - d->insertEnd(loadIndex, startTime - d->range(loadIndex).start); - } else { - // if it's a new entry it means that we don't have a corresponding start - newEvent.pixmapEventType = PixmapLoadingStarted; - newEvent.rowNumberExpanded = newEvent.urlIndex + 2; - loadIndex = d->insert(traceStartTime(), startTime - traceStartTime(), newEvent); - pixmapStartPoints[newEvent.urlIndex] = loadIndex; + // First try to find one that has already started + for (QVector::const_iterator i(pixmap.sizes.cbegin()); + i != pixmap.sizes.cend(); ++i) { + if (i->loadState != Loading) + continue; + // Pixmaps with known size cannot be errors and vice versa + if (newEvent.pixmapEventType == PixmapLoadingError && i->size.isValid()) + continue; + + newEvent.sizeIndex = i - pixmap.sizes.cbegin(); + break; + } + + // If none was found use any other compatible one + if (newEvent.sizeIndex == -1) { + for (QVector::const_iterator i(pixmap.sizes.cbegin()); + i != pixmap.sizes.cend(); ++i) { + if (i->loadState != Initial) + continue; + // Pixmaps with known size cannot be errors and vice versa + if (newEvent.pixmapEventType == PixmapLoadingError && i->size.isValid()) + continue; + + newEvent.sizeIndex = i - pixmap.sizes.cbegin(); + break; + } + } + + // If again none was found, create one. + if (newEvent.sizeIndex == -1) { + newEvent.sizeIndex = pixmap.sizes.length(); + pixmap.sizes << PixmapState(); + } + + PixmapState &state = pixmap.sizes[newEvent.sizeIndex]; + // If the pixmap loading wasn't started, start it at traceStartTime() + if (state.loadState == Initial) { + newEvent.pixmapEventType = PixmapLoadingStarted; + state.started = d->insert(traceStartTime(), startTime - traceStartTime(), newEvent); + } + + d->insertEnd(state.started, startTime - d->range(state.started).start); + if (newEvent.pixmapEventType == PixmapLoadingError) { + state.loadState = Error; + switch (state.cacheState) { + case Uncached: + state.cacheState = Uncacheable; + break; + case ToBeCached: + state.cacheState = Corrupt; + break; + default: + // Cached cannot happen as size would have to be known and Corrupt or + // Uncacheable cannot happen as we only accept one finish or error event per + // pixmap. + Q_ASSERT(false); + } + } else { + state.loadState = Finished; } - if (event.bindingType == PixmapLoadingFinished) - d->data(loadIndex).cacheSize = 1; // use count to mark success - else - d->data(loadIndex).cacheSize = -1; // ... or failure break; } default: @@ -306,8 +496,7 @@ void PixmapCacheModel::clear() { Q_D(PixmapCacheModel); d->SortedTimelineModel::clear(); - d->pixmapUrls.clear(); - d->pixmapSizes.clear(); + d->pixmaps.clear(); d->collapsedRowCount = 1; d->expandedRowCount = 1; d->expanded = false; @@ -380,6 +569,23 @@ void PixmapCacheModel::PixmapCacheModelPrivate::computeRowCounts() collapsedRowCount++; } +int PixmapCacheModel::PixmapCacheModelPrivate::updateCacheCount(int lastCacheSizeEvent, + qint64 startTime, qint64 pixSize, PixmapCacheEvent &newEvent) +{ + newEvent.pixmapEventType = PixmapCacheCountChanged; + newEvent.eventId = 0; + newEvent.rowNumberExpanded = 1; + newEvent.rowNumberCollapsed = 1; + + qint64 prevSize = 0; + if (lastCacheSizeEvent != -1) { + prevSize = range(lastCacheSizeEvent).cacheSize; + insertEnd(lastCacheSizeEvent, startTime - range(lastCacheSizeEvent).start); + } + + newEvent.cacheSize = prevSize + pixSize; + return insertStart(startTime, newEvent); +} } // namespace Internal diff --git a/plugins/qmlprofilerextension/pixmapcachemodel.h b/plugins/qmlprofilerextension/pixmapcachemodel.h index b0470f37130..cbebf71d0d6 100644 --- a/plugins/qmlprofilerextension/pixmapcachemodel.h +++ b/plugins/qmlprofilerextension/pixmapcachemodel.h @@ -38,6 +38,7 @@ public: int eventId; int pixmapEventType; int urlIndex; + int sizeIndex; qint64 cacheSize; int rowNumberExpanded; int rowNumberCollapsed; From b4d54648c4a25771d9450cb78ca38a2d6cfe1b69 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Tue, 25 Mar 2014 12:06:48 +0100 Subject: [PATCH 5/7] Don't save minCacheSize and properly initialize maxCacheSize minCacheSize wasn't used anywhere and a maxCacheSize <= 0 is useless and may lead to wrong height values for cache events. Change-Id: Iee8c95197a7851bd41238e8e465bf6fd5b994573 Reviewed-by: Kai Koehne --- .../qmlprofilerextension/pixmapcachemodel.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/plugins/qmlprofilerextension/pixmapcachemodel.cpp b/plugins/qmlprofilerextension/pixmapcachemodel.cpp index f5d08083e83..7e04f51b391 100644 --- a/plugins/qmlprofilerextension/pixmapcachemodel.cpp +++ b/plugins/qmlprofilerextension/pixmapcachemodel.cpp @@ -67,7 +67,7 @@ class PixmapCacheModel::PixmapCacheModelPrivate : SingleCategoryTimelineModel::SingleCategoryTimelineModelPrivate> { public: - void computeCacheSizes(); + void computeMaxCacheSize(); void resizeUnfinishedLoads(); void flattenLoads(); void computeRowCounts(); @@ -79,7 +79,6 @@ public: int collapsedRowCount; void addVP(QVariantList &l, QString label, qint64 time) const; - qint64 minCacheSize; qint64 maxCacheSize; private: Q_DECLARE_PUBLIC(PixmapCacheModel) @@ -93,6 +92,7 @@ PixmapCacheModel::PixmapCacheModel(QObject *parent) Q_D(PixmapCacheModel); d->collapsedRowCount = 1; d->expandedRowCount = 1; + d->maxCacheSize = 1; } int PixmapCacheModel::categoryDepth(int categoryIndex) const @@ -484,7 +484,7 @@ void PixmapCacheModel::loadData() d->resizeUnfinishedLoads(); - d->computeCacheSizes(); + d->computeMaxCacheSize(); d->flattenLoads(); d->computeRowCounts(); d->computeNesting(); @@ -499,20 +499,18 @@ void PixmapCacheModel::clear() d->pixmaps.clear(); d->collapsedRowCount = 1; d->expandedRowCount = 1; + d->maxCacheSize = 1; d->expanded = false; d->modelManager->modelProxyCountUpdated(d->modelId, 0, 1); } -void PixmapCacheModel::PixmapCacheModelPrivate::computeCacheSizes() +void PixmapCacheModel::PixmapCacheModelPrivate::computeMaxCacheSize() { - minCacheSize = -1; - maxCacheSize = -1; + maxCacheSize = 1; foreach (const PixmapCacheModel::PixmapCacheEvent &event, ranges) { if (event.pixmapEventType == PixmapCacheModel::PixmapCacheCountChanged) { - if (minCacheSize == -1 || event.cacheSize < minCacheSize) - minCacheSize = event.cacheSize; - if (maxCacheSize == -1 || event.cacheSize > maxCacheSize) + if (event.cacheSize > maxCacheSize) maxCacheSize = event.cacheSize; } } From 9b92cf8ed1ae6fa5e3c3edd94d8a9153eebb31e6 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Thu, 27 Mar 2014 15:43:09 +0100 Subject: [PATCH 6/7] Remove some dead code Keeping redundant copies of the same data is wasteful and error prone. Change-Id: Ie4273d01728acd0caecf20739ae53376ef91ace8 Reviewed-by: Kai Koehne --- .../qmlprofilerextension/pixmapcachemodel.cpp | 30 ++++--------------- .../qmlprofilerextension/pixmapcachemodel.h | 4 +-- 2 files changed, 7 insertions(+), 27 deletions(-) diff --git a/plugins/qmlprofilerextension/pixmapcachemodel.cpp b/plugins/qmlprofilerextension/pixmapcachemodel.cpp index 7e04f51b391..e3c65c68f72 100644 --- a/plugins/qmlprofilerextension/pixmapcachemodel.cpp +++ b/plugins/qmlprofilerextension/pixmapcachemodel.cpp @@ -70,12 +70,10 @@ public: void computeMaxCacheSize(); void resizeUnfinishedLoads(); void flattenLoads(); - void computeRowCounts(); int updateCacheCount(int lastCacheSizeEvent, qint64 startTime, qint64 pixSize, PixmapCacheEvent &newEvent); QVector pixmaps; - int expandedRowCount; int collapsedRowCount; void addVP(QVariantList &l, QString label, qint64 time) const; @@ -91,7 +89,6 @@ PixmapCacheModel::PixmapCacheModel(QObject *parent) { Q_D(PixmapCacheModel); d->collapsedRowCount = 1; - d->expandedRowCount = 1; d->maxCacheSize = 1; } @@ -102,7 +99,7 @@ int PixmapCacheModel::categoryDepth(int categoryIndex) const if (isEmpty()) return 1; if (d->expanded) - return d->expandedRowCount; + return d->pixmaps.count() + 2; return d->collapsedRowCount; } @@ -110,14 +107,15 @@ int PixmapCacheModel::getEventRow(int index) const { Q_D(const PixmapCacheModel); if (d->expanded) - return d->range(index).rowNumberExpanded; + return getEventId(index) + 1; return d->range(index).rowNumberCollapsed; } int PixmapCacheModel::getEventId(int index) const { Q_D(const PixmapCacheModel); - return d->range(index).eventId; + return d->range(index).pixmapEventType == PixmapCacheCountChanged ? + 0 : d->range(index).urlIndex + 1; } QColor PixmapCacheModel::getColor(int index) const @@ -297,9 +295,6 @@ void PixmapCacheModel::loadData() d->pixmaps << Pixmap(event.location.filename); } - newEvent.eventId = newEvent.urlIndex + 1; - newEvent.rowNumberExpanded = newEvent.urlIndex + 2; - Pixmap &pixmap = d->pixmaps[newEvent.urlIndex]; switch (newEvent.pixmapEventType) { case PixmapSizeKnown: {// pixmap size @@ -486,7 +481,6 @@ void PixmapCacheModel::loadData() d->computeMaxCacheSize(); d->flattenLoads(); - d->computeRowCounts(); d->computeNesting(); d->modelManager->modelProxyCountUpdated(d->modelId, 1, 1); @@ -498,7 +492,6 @@ void PixmapCacheModel::clear() d->SortedTimelineModel::clear(); d->pixmaps.clear(); d->collapsedRowCount = 1; - d->expandedRowCount = 1; d->maxCacheSize = 1; d->expanded = false; @@ -530,6 +523,8 @@ void PixmapCacheModel::PixmapCacheModelPrivate::resizeUnfinishedLoads() void PixmapCacheModel::PixmapCacheModelPrivate::flattenLoads() { + collapsedRowCount = 0; + // computes "compressed row" QVector eventEndTimes; for (int i = 0; i < count(); i++) { @@ -548,22 +543,11 @@ void PixmapCacheModel::PixmapCacheModelPrivate::flattenLoads() // readjust to account for category empty row and bargraph event.rowNumberCollapsed += 2; } - } -} - -void PixmapCacheModel::PixmapCacheModelPrivate::computeRowCounts() -{ - expandedRowCount = 0; - collapsedRowCount = 0; - foreach (const PixmapCacheModel::PixmapCacheEvent &event, ranges) { - if (event.rowNumberExpanded > expandedRowCount) - expandedRowCount = event.rowNumberExpanded; if (event.rowNumberCollapsed > collapsedRowCount) collapsedRowCount = event.rowNumberCollapsed; } // Starting from 0, count is maxIndex+1 - expandedRowCount++; collapsedRowCount++; } @@ -571,8 +555,6 @@ int PixmapCacheModel::PixmapCacheModelPrivate::updateCacheCount(int lastCacheSiz qint64 startTime, qint64 pixSize, PixmapCacheEvent &newEvent) { newEvent.pixmapEventType = PixmapCacheCountChanged; - newEvent.eventId = 0; - newEvent.rowNumberExpanded = 1; newEvent.rowNumberCollapsed = 1; qint64 prevSize = 0; diff --git a/plugins/qmlprofilerextension/pixmapcachemodel.h b/plugins/qmlprofilerextension/pixmapcachemodel.h index cbebf71d0d6..fdf20f51951 100644 --- a/plugins/qmlprofilerextension/pixmapcachemodel.h +++ b/plugins/qmlprofilerextension/pixmapcachemodel.h @@ -35,13 +35,11 @@ class PixmapCacheModel : public QmlProfiler::SingleCategoryTimelineModel public: struct PixmapCacheEvent { - int eventId; int pixmapEventType; int urlIndex; int sizeIndex; - qint64 cacheSize; - int rowNumberExpanded; int rowNumberCollapsed; + qint64 cacheSize; }; enum PixmapEventType { From b69bedbf0713a0fb1b42a78215b7feb8cb7b7982 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Wed, 9 Apr 2014 16:33:00 +0200 Subject: [PATCH 7/7] No need to qualify calls to SortedTimelineModel::clear() Task-number: QTCREATORBUG-12010 Change-Id: I22bc4525329f50359e6857c595fe686af901711e Reviewed-by: Kai Koehne --- plugins/qmlprofilerextension/pixmapcachemodel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/qmlprofilerextension/pixmapcachemodel.cpp b/plugins/qmlprofilerextension/pixmapcachemodel.cpp index e3c65c68f72..f329963449d 100644 --- a/plugins/qmlprofilerextension/pixmapcachemodel.cpp +++ b/plugins/qmlprofilerextension/pixmapcachemodel.cpp @@ -489,7 +489,7 @@ void PixmapCacheModel::loadData() void PixmapCacheModel::clear() { Q_D(PixmapCacheModel); - d->SortedTimelineModel::clear(); + d->clear(); d->pixmaps.clear(); d->collapsedRowCount = 1; d->maxCacheSize = 1;