From 47ce17b1ba82e396480c565dda6334fc58ed5cc3 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Wed, 12 Feb 2014 17:35:08 +0100 Subject: [PATCH] QmlProfiler: Sanitize the signal exchange between models a bit The model manager should only set its state to 'Done' if all models are actually done. When that is the case it can safely emit dataAvailable, too, freeing us of the need to apply a heuristic to the progress percentage. In order to have a unified interface to the completion of model processing an abstract base class for QML and V8 models is introduced. Task-number: QTCREATORBUG-11466 Change-Id: Id89c7ef5e24004baab7f37ee5486b69e7611aee0 Reviewed-by: Christian Stenger Reviewed-by: Kai Koehne --- .../qmlprofiler/abstracttimelinemodel.cpp | 2 +- src/plugins/qmlprofiler/qmlprofiler.pro | 6 +- src/plugins/qmlprofiler/qmlprofiler.qbs | 1 + .../qmlprofiler/qmlprofilerbasemodel.cpp | 57 +++++++++++++++++ .../qmlprofiler/qmlprofilerbasemodel.h | 62 +++++++++++++++++++ .../qmlprofilereventsmodelproxy.cpp | 2 +- .../qmlprofiler/qmlprofilermodelmanager.cpp | 30 ++++++--- .../qmlprofiler/qmlprofilermodelmanager.h | 1 + .../qmlprofiler/qmlprofilerprocessedmodel.cpp | 22 +++---- .../qmlprofiler/qmlprofilerprocessedmodel.h | 3 +- .../qmlprofiler/qmlprofilersimplemodel.cpp | 16 +---- .../qmlprofiler/qmlprofilersimplemodel.h | 11 +--- .../qmlprofiler/qmlprofilerstatewidget.cpp | 2 +- .../qmlprofiler/qv8profilerdatamodel.cpp | 9 +-- .../qmlprofiler/qv8profilerdatamodel.h | 9 ++- 15 files changed, 171 insertions(+), 62 deletions(-) create mode 100644 src/plugins/qmlprofiler/qmlprofilerbasemodel.cpp create mode 100644 src/plugins/qmlprofiler/qmlprofilerbasemodel.h diff --git a/src/plugins/qmlprofiler/abstracttimelinemodel.cpp b/src/plugins/qmlprofiler/abstracttimelinemodel.cpp index a4823b3582f..53277f78e96 100644 --- a/src/plugins/qmlprofiler/abstracttimelinemodel.cpp +++ b/src/plugins/qmlprofiler/abstracttimelinemodel.cpp @@ -81,7 +81,7 @@ int AbstractTimelineModel::getBindingLoopDest(int index) const void AbstractTimelineModel::dataChanged() { switch (m_modelManager->state()) { - case QmlProfilerDataState::Done: + case QmlProfilerDataState::ProcessingData: loadData(); break; case QmlProfilerDataState::ClearingData: diff --git a/src/plugins/qmlprofiler/qmlprofiler.pro b/src/plugins/qmlprofiler/qmlprofiler.pro index 8ca0e82f5a6..3b5d8d54922 100644 --- a/src/plugins/qmlprofiler/qmlprofiler.pro +++ b/src/plugins/qmlprofiler/qmlprofiler.pro @@ -31,7 +31,8 @@ SOURCES += \ abstracttimelinemodel.cpp \ timelinemodelaggregator.cpp \ qmlprofilerpainteventsmodelproxy.cpp \ - sortedtimelinemodel.cpp + sortedtimelinemodel.cpp \ + qmlprofilerbasemodel.cpp HEADERS += \ qmlprofilerconstants.h \ @@ -63,7 +64,8 @@ HEADERS += \ abstracttimelinemodel.h \ timelinemodelaggregator.h \ qmlprofilerpainteventsmodelproxy.h \ - sortedtimelinemodel.h + sortedtimelinemodel.h \ + qmlprofilerbasemodel.h RESOURCES += \ qml/qmlprofiler.qrc diff --git a/src/plugins/qmlprofiler/qmlprofiler.qbs b/src/plugins/qmlprofiler/qmlprofiler.qbs index a06f46beffb..40fcc0354ef 100644 --- a/src/plugins/qmlprofiler/qmlprofiler.qbs +++ b/src/plugins/qmlprofiler/qmlprofiler.qbs @@ -28,6 +28,7 @@ QtcPlugin { "localqmlprofilerrunner.cpp", "localqmlprofilerrunner.h", "qmlprofiler_global.h", "qmlprofilerattachdialog.cpp", "qmlprofilerattachdialog.h", + "qmlprofilerbasemodel.cpp", "qmlprofilerbasemodel.h", "qmlprofilerclientmanager.cpp", "qmlprofilerclientmanager.h", "qmlprofilerconstants.h", "qmlprofilerdetailsrewriter.cpp", "qmlprofilerdetailsrewriter.h", diff --git a/src/plugins/qmlprofiler/qmlprofilerbasemodel.cpp b/src/plugins/qmlprofiler/qmlprofilerbasemodel.cpp new file mode 100644 index 00000000000..2ea14f9db5c --- /dev/null +++ b/src/plugins/qmlprofiler/qmlprofilerbasemodel.cpp @@ -0,0 +1,57 @@ +/**************************************************************************** +** +** Copyright (C) 2014 Digia Plc and/or its subsidiary(-ies). +** Contact: http://www.qt-project.org/legal +** +** This file is part of Qt Creator. +** +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and Digia. For licensing terms and +** conditions see http://qt.digia.com/licensing. For further information +** use the contact form at http://qt.digia.com/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 2.1 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 2.1 requirements +** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Digia gives you certain additional +** rights. These rights are described in the Digia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +****************************************************************************/ + +#include "qmlprofilerbasemodel.h" +#include "qmlprofilermodelmanager.h" + +namespace QmlProfiler { + +QmlProfilerBaseModel::QmlProfilerBaseModel(QmlProfilerModelManager *manager) : + m_modelManager(manager), m_processingDone(false) +{ + Q_ASSERT(m_modelManager); + m_modelId = m_modelManager->registerModelProxy(); +} + +void QmlProfilerBaseModel::clear() +{ + m_modelManager->modelProxyCountUpdated(m_modelId, 0, 1); + m_processingDone = false; + emit changed(); +} + +void QmlProfilerBaseModel::complete() +{ + emit changed(); + m_processingDone = true; + m_modelManager->modelProxyCountUpdated(m_modelId, isEmpty() ? 0 : 1, 1); + m_modelManager->modelProcessingDone(); +} + +} diff --git a/src/plugins/qmlprofiler/qmlprofilerbasemodel.h b/src/plugins/qmlprofiler/qmlprofilerbasemodel.h new file mode 100644 index 00000000000..b565ea194b3 --- /dev/null +++ b/src/plugins/qmlprofiler/qmlprofilerbasemodel.h @@ -0,0 +1,62 @@ +/**************************************************************************** +** +** Copyright (C) 2014 Digia Plc and/or its subsidiary(-ies). +** Contact: http://www.qt-project.org/legal +** +** This file is part of Qt Creator. +** +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and Digia. For licensing terms and +** conditions see http://qt.digia.com/licensing. For further information +** use the contact form at http://qt.digia.com/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 2.1 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 2.1 requirements +** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Digia gives you certain additional +** rights. These rights are described in the Digia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +****************************************************************************/ + +#ifndef QMLPROFILERBASEMODEL_H +#define QMLPROFILERBASEMODEL_H + +#include "qmlprofiler_global.h" +#include + +namespace QmlProfiler { + +class QmlProfilerModelManager; + +class QMLPROFILER_EXPORT QmlProfilerBaseModel : public QObject { + Q_OBJECT +public: + QmlProfilerBaseModel(QmlProfilerModelManager *manager); + virtual ~QmlProfilerBaseModel() {} + + virtual void complete(); + virtual void clear(); + virtual bool isEmpty() const = 0; + bool processingDone() const { return m_processingDone; } + +protected: + QmlProfilerModelManager *m_modelManager; + int m_modelId; + bool m_processingDone; + +signals: + void changed(); +}; + +} + +#endif // QMLPROFILERBASEMODEL_H diff --git a/src/plugins/qmlprofiler/qmlprofilereventsmodelproxy.cpp b/src/plugins/qmlprofiler/qmlprofilereventsmodelproxy.cpp index 9513d2cc460..19cc03223c7 100644 --- a/src/plugins/qmlprofiler/qmlprofilereventsmodelproxy.cpp +++ b/src/plugins/qmlprofiler/qmlprofilereventsmodelproxy.cpp @@ -96,7 +96,7 @@ void QmlProfilerEventsModelProxy::limitToRange(qint64 rangeStart, qint64 rangeEn void QmlProfilerEventsModelProxy::dataChanged() { - if (d->modelManager->state() == QmlProfilerDataState::Done) + if (d->modelManager->state() == QmlProfilerDataState::ProcessingData) loadData(); else if (d->modelManager->state() == QmlProfilerDataState::ClearingData) clear(); diff --git a/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp b/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp index 62751f3d222..abedd2700f4 100644 --- a/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp +++ b/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp @@ -216,8 +216,6 @@ void QmlProfilerModelManager::modelProxyCountUpdated(int proxyId, qint64 count, d->progress += d->partialCounts[proxyId] / d->partialCounts.count(); emit progressChanged(); - if (d->progress > 0.99) - emit dataAvailable(); } qint64 QmlProfilerModelManager::estimatedProfilingTime() const @@ -259,25 +257,37 @@ void QmlProfilerModelManager::addV8Event(int depth, const QString &function, con void QmlProfilerModelManager::complete() { - if (state() == QmlProfilerDataState::AcquiringData) { + switch (state()) { + case QmlProfilerDataState::ProcessingData: + setState(QmlProfilerDataState::Done); + emit dataAvailable(); + break; + case QmlProfilerDataState::AcquiringData: // If trace end time was not explicitly set, use the last event if (d->traceTime->endTime() == 0) d->traceTime->setEndTime(d->model->lastTimeMark()); setState(QmlProfilerDataState::ProcessingData); d->model->complete(); d->v8Model->complete(); + break; + case QmlProfilerDataState::Empty: setState(QmlProfilerDataState::Done); - } else - if (state() == QmlProfilerDataState::Empty) { - setState(QmlProfilerDataState::Done); - } else - if (state() == QmlProfilerDataState::Done) { - // repeated Done states are ignored - } else { + break; + case QmlProfilerDataState::Done: + break; + default: emit error(tr("Unexpected complete signal in data model.")); + break; } } +void QmlProfilerModelManager::modelProcessingDone() +{ + Q_ASSERT(state() == QmlProfilerDataState::ProcessingData); + if (d->model->processingDone() && d->v8Model->processingDone()) + complete(); +} + void QmlProfilerModelManager::save(const QString &filename) { QFile file(filename); diff --git a/src/plugins/qmlprofiler/qmlprofilermodelmanager.h b/src/plugins/qmlprofiler/qmlprofilermodelmanager.h index 146c6acdfd2..92acb8fb8de 100644 --- a/src/plugins/qmlprofiler/qmlprofilermodelmanager.h +++ b/src/plugins/qmlprofiler/qmlprofilermodelmanager.h @@ -141,6 +141,7 @@ public slots: double totalTime, double selfTime); void complete(); + void modelProcessingDone(); void save(const QString &filename); void load(const QString &filename); diff --git a/src/plugins/qmlprofiler/qmlprofilerprocessedmodel.cpp b/src/plugins/qmlprofiler/qmlprofilerprocessedmodel.cpp index 7cf5829f46d..240814a125b 100644 --- a/src/plugins/qmlprofiler/qmlprofilerprocessedmodel.cpp +++ b/src/plugins/qmlprofiler/qmlprofilerprocessedmodel.cpp @@ -28,6 +28,7 @@ ****************************************************************************/ #include "qmlprofilerprocessedmodel.h" +#include "qmlprofilermodelmanager.h" #include #include #include @@ -96,10 +97,9 @@ bool compareStartTimes(const QmlProfilerSimpleModel::QmlEventData &t1, const Qml ////////////////////////////////////////////////////////////////////////////// -QmlProfilerProcessedModel::QmlProfilerProcessedModel(Utils::FileInProjectFinder *fileFinder, QObject *parent) +QmlProfilerProcessedModel::QmlProfilerProcessedModel(Utils::FileInProjectFinder *fileFinder, QmlProfilerModelManager *parent) : QmlProfilerSimpleModel(parent) , m_detailsRewriter(new QmlProfilerDetailsRewriter(this, fileFinder)) - , m_emitChanged(false) { connect(m_detailsRewriter, SIGNAL(rewriteDetailsString(int,QString)), this, SLOT(detailsChanged(int,QString))); @@ -117,8 +117,6 @@ void QmlProfilerProcessedModel::clear() // This call emits changed(). Don't emit it again here. QmlProfilerSimpleModel::clear(); - - m_emitChanged = false; } void QmlProfilerProcessedModel::complete() @@ -154,12 +152,9 @@ void QmlProfilerProcessedModel::complete() m_detailsRewriter->requestDetailsForLocation(i, event->location); } + // Allow QmlProfilerBaseModel::complete() only after documents have been reloaded to avoid + // unnecessary updates of child models. m_detailsRewriter->reloadDocuments(); - - // This call emits changed(). Don't emit it again here. - QmlProfilerSimpleModel::complete(); - - m_emitChanged = false; } void QmlProfilerProcessedModel::detailsChanged(int requestId, const QString &newString) @@ -168,16 +163,13 @@ void QmlProfilerProcessedModel::detailsChanged(int requestId, const QString &new QmlEventData *event = &eventList[requestId]; event->data = QStringList(newString); - - m_emitChanged = true; } void QmlProfilerProcessedModel::detailsDone() { - if (m_emitChanged) { - emit changed(); - m_emitChanged = false; - } + // The child models are supposed to synchronously update on changed(), triggered by + // QmlProfilerBaseModel::complete(). + QmlProfilerSimpleModel::complete(); } } diff --git a/src/plugins/qmlprofiler/qmlprofilerprocessedmodel.h b/src/plugins/qmlprofiler/qmlprofilerprocessedmodel.h index bbbb4a7889f..3e3738824dd 100644 --- a/src/plugins/qmlprofiler/qmlprofilerprocessedmodel.h +++ b/src/plugins/qmlprofiler/qmlprofilerprocessedmodel.h @@ -41,7 +41,7 @@ class QmlProfilerProcessedModel : public QmlProfilerSimpleModel Q_OBJECT public: - explicit QmlProfilerProcessedModel(Utils::FileInProjectFinder *fileFinder, QObject *parent = 0); + explicit QmlProfilerProcessedModel(Utils::FileInProjectFinder *fileFinder, QmlProfilerModelManager *parent = 0); ~QmlProfilerProcessedModel(); virtual void clear(); @@ -53,7 +53,6 @@ private slots: private: QmlProfilerDetailsRewriter *m_detailsRewriter; - bool m_emitChanged; }; } diff --git a/src/plugins/qmlprofiler/qmlprofilersimplemodel.cpp b/src/plugins/qmlprofiler/qmlprofilersimplemodel.cpp index d509af267ad..93759ec14b9 100644 --- a/src/plugins/qmlprofiler/qmlprofilersimplemodel.cpp +++ b/src/plugins/qmlprofiler/qmlprofilersimplemodel.cpp @@ -39,12 +39,9 @@ namespace QmlProfiler { -QmlProfilerSimpleModel::QmlProfilerSimpleModel(QObject *parent) - : QObject(parent) +QmlProfilerSimpleModel::QmlProfilerSimpleModel(QmlProfilerModelManager *parent) + : QmlProfilerBaseModel(parent) { - m_modelManager = qobject_cast(parent); - Q_ASSERT(m_modelManager); - m_modelId = m_modelManager->registerModelProxy(); } QmlProfilerSimpleModel::~QmlProfilerSimpleModel() @@ -53,9 +50,8 @@ QmlProfilerSimpleModel::~QmlProfilerSimpleModel() void QmlProfilerSimpleModel::clear() { - m_modelManager->modelProxyCountUpdated(m_modelId, 0, 1); eventList.clear(); - emit changed(); + QmlProfilerBaseModel::clear(); } bool QmlProfilerSimpleModel::isEmpty() const @@ -98,12 +94,6 @@ qint64 QmlProfilerSimpleModel::lastTimeMark() const return eventList.last().startTime + eventList.last().duration; } -void QmlProfilerSimpleModel::complete() -{ - m_modelManager->modelProxyCountUpdated(m_modelId, isEmpty() ? 0 : 1, 1); - emit changed(); -} - QString QmlProfilerSimpleModel::getHashString(const QmlProfilerSimpleModel::QmlEventData &event) { return QString::fromLatin1("%1:%2:%3:%4:%5").arg( diff --git a/src/plugins/qmlprofiler/qmlprofilersimplemodel.h b/src/plugins/qmlprofiler/qmlprofilersimplemodel.h index 88421107f7d..afdc6bc5056 100644 --- a/src/plugins/qmlprofiler/qmlprofilersimplemodel.h +++ b/src/plugins/qmlprofiler/qmlprofilersimplemodel.h @@ -31,6 +31,7 @@ #define QMLPROFILERSIMPLEMODEL_H #include "qmlprofiler_global.h" +#include "qmlprofilerbasemodel.h" #include @@ -43,7 +44,7 @@ namespace QmlProfiler { class QmlProfilerModelManager; // stores the data from the client as-is -class QMLPROFILER_EXPORT QmlProfilerSimpleModel : public QObject +class QMLPROFILER_EXPORT QmlProfilerSimpleModel : public QmlProfilerBaseModel { Q_OBJECT public: @@ -62,7 +63,7 @@ public: qint64 numericData5; }; - explicit QmlProfilerSimpleModel(QObject *parent = 0); + explicit QmlProfilerSimpleModel(QmlProfilerModelManager *parent); ~QmlProfilerSimpleModel(); virtual void clear(); @@ -72,17 +73,11 @@ public: void addQmlEvent(int type, int bindingType, qint64 startTime, qint64 duration, const QStringList &data, const QmlDebug::QmlEventLocation &location, qint64 ndata1, qint64 ndata2, qint64 ndata3, qint64 ndata4, qint64 ndata5); qint64 lastTimeMark() const; - virtual void complete(); static QString getHashString(const QmlProfilerSimpleModel::QmlEventData &event); -signals: - void changed(); - protected: QVector eventList; - QmlProfilerModelManager *m_modelManager; - int m_modelId; }; } diff --git a/src/plugins/qmlprofiler/qmlprofilerstatewidget.cpp b/src/plugins/qmlprofiler/qmlprofilerstatewidget.cpp index bd2becee4cc..c6eb7f10380 100644 --- a/src/plugins/qmlprofiler/qmlprofilerstatewidget.cpp +++ b/src/plugins/qmlprofiler/qmlprofilerstatewidget.cpp @@ -261,7 +261,7 @@ void QmlProfilerStateWidget::updateDisplay() void QmlProfilerStateWidget::dataStateChanged() { // consider possible rounding errors - d->loadingDone = d->m_modelManager->progress() >= 0.99 || + d->loadingDone = d->m_modelManager->state() == QmlProfilerDataState::Done || d->m_modelManager->state() == QmlProfilerDataState::Empty; d->traceAvailable = d->m_modelManager->traceTime()->duration() > 0; d->emptyList = d->m_modelManager->isEmpty() || d->m_modelManager->progress() == 0; diff --git a/src/plugins/qmlprofiler/qv8profilerdatamodel.cpp b/src/plugins/qmlprofiler/qv8profilerdatamodel.cpp index d6899b65a5e..398f5fc25e1 100644 --- a/src/plugins/qmlprofiler/qv8profilerdatamodel.cpp +++ b/src/plugins/qmlprofiler/qv8profilerdatamodel.cpp @@ -28,6 +28,7 @@ ****************************************************************************/ #include "qv8profilerdatamodel.h" +#include "qmlprofilermodelmanager.h" #include @@ -107,8 +108,8 @@ public: qint64 v8MeasuredTime; }; -QV8ProfilerDataModel::QV8ProfilerDataModel(QObject *parent) - : QObject(parent) +QV8ProfilerDataModel::QV8ProfilerDataModel(QmlProfilerModelManager *parent) + : QmlProfilerBaseModel(parent) , d(new QV8ProfilerDataModelPrivate(this)) { d->v8MeasuredTime = 0; @@ -128,7 +129,7 @@ void QV8ProfilerDataModel::clear() d->clearV8RootEvent(); d->v8MeasuredTime = 0; - emit changed(); + QmlProfilerBaseModel::clear(); } bool QV8ProfilerDataModel::isEmpty() const @@ -482,7 +483,7 @@ void QV8ProfilerDataModel::load(QXmlStreamReader &stream) void QV8ProfilerDataModel::complete() { collectV8Statistics(); - emit changed(); + QmlProfilerBaseModel::complete(); } } // namespace Internal diff --git a/src/plugins/qmlprofiler/qv8profilerdatamodel.h b/src/plugins/qmlprofiler/qv8profilerdatamodel.h index d9579f35cc5..c2b7995a0ae 100644 --- a/src/plugins/qmlprofiler/qv8profilerdatamodel.h +++ b/src/plugins/qmlprofiler/qv8profilerdatamodel.h @@ -30,6 +30,8 @@ #ifndef QV8PROFILERDATAMODEL_H #define QV8PROFILERDATAMODEL_H +#include "qmlprofilerbasemodel.h" + #include #include @@ -70,11 +72,11 @@ struct QV8EventSub { qint64 totalTime; }; -class QV8ProfilerDataModel : public QObject +class QV8ProfilerDataModel : public QmlProfilerBaseModel { Q_OBJECT public: - QV8ProfilerDataModel(QObject *parent = 0); + QV8ProfilerDataModel(QmlProfilerModelManager *parent); ~QV8ProfilerDataModel(); void clear(); @@ -90,9 +92,6 @@ public: void complete(); -signals: - void changed(); - public slots: void addV8Event(int depth, const QString &function,