Tracing: Don't restore notes for events that have been filtered out

Change-Id: I4dc13e579e7994bb970d121a2baa8169a8dd1b2b
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
This commit is contained in:
Ulf Hermann
2018-05-08 10:05:35 +02:00
parent aa3c9fc135
commit 4d8707afa4
3 changed files with 37 additions and 21 deletions

View File

@@ -76,7 +76,7 @@ int TimelineNotesModel::typeId(int index) const
Q_D(const TimelineNotesModel); Q_D(const TimelineNotesModel);
const TimelineNotesModelPrivate::Note &note = d->data[index]; const TimelineNotesModelPrivate::Note &note = d->data[index];
const TimelineModel *model = timelineModelByModelId(note.timelineModel); const TimelineModel *model = timelineModelByModelId(note.timelineModel);
if (!model) if (!model || note.timelineIndex >= model->count())
return -1; // This can happen if one of the timeline models has been removed return -1; // This can happen if one of the timeline models has been removed
return model->typeId(note.timelineIndex); return model->typeId(note.timelineIndex);
} }

View File

@@ -110,6 +110,7 @@ void QmlProfilerNotesModel::stash()
continue; continue;
int index = timelineIndex(i); int index = timelineIndex(i);
if (index < model->count()) {
QmlNote save = { QmlNote save = {
model->typeId(index), model->typeId(index),
model->collapsedRow(index), model->collapsedRow(index),
@@ -119,6 +120,7 @@ void QmlProfilerNotesModel::stash()
}; };
m_notes.append(save); m_notes.append(save);
} }
}
resetModified(); resetModified();
} }

View File

@@ -71,15 +71,18 @@ int FlameGraphModelTest::generateData(QmlProfilerModelManager *manager,
} }
event.setRangeStage(RangeEnd); event.setRangeStage(RangeEnd);
event.setTypeIndex(typeIndices.pop());
event.setTimestamp(++i); event.setTimestamp(++i);
manager->addEvent(event); manager->addEvent(event);
event.setRangeStage(RangeStart); event.setRangeStage(RangeStart);
event.setTypeIndex(0); // Have it accepted by the JavaScript range model above
typeIndices.push(0);
event.setTimestamp(++i); event.setTimestamp(++i);
manager->addEvent(event); manager->addEvent(event);
for (int j = 0; !typeIndices.isEmpty(); ++j) { while (!typeIndices.isEmpty()) {
event.setTimestamp(i + j); event.setTimestamp(++i);
event.setRangeStage(RangeEnd); event.setRangeStage(RangeEnd);
event.setTypeIndex(typeIndices.pop()); event.setTypeIndex(typeIndices.pop());
manager->addEvent(event); manager->addEvent(event);
@@ -88,7 +91,11 @@ int FlameGraphModelTest::generateData(QmlProfilerModelManager *manager,
manager->finalize(); manager->finalize();
static_cast<QmlProfilerNotesModel *>(manager->notesModel()) static_cast<QmlProfilerNotesModel *>(manager->notesModel())
->setNotes(QVector<QmlNote>({QmlNote(0, 2, 1, 20, "dings")})); ->setNotes(QVector<QmlNote>({
// row 2 on purpose to test the range heuristic
QmlNote(0, 2, 1, 21, "dings"),
QmlNote(0, 3, 12, 1, "weg")
}));
manager->notesModel()->restore(); manager->notesModel()->restore();
return rangeModelId; return rangeModelId;
@@ -138,8 +145,8 @@ void FlameGraphModelTest::testData()
FlameGraphModel::tr("JavaScript")); FlameGraphModel::tr("JavaScript"));
QCOMPARE(model.data(index2, FlameGraphModel::TypeRole).toString(), QCOMPARE(model.data(index2, FlameGraphModel::TypeRole).toString(),
FlameGraphModel::tr("Compile")); FlameGraphModel::tr("Compile"));
QCOMPARE(model.data(index, FlameGraphModel::DurationRole).toLongLong(), 20); QCOMPARE(model.data(index, FlameGraphModel::DurationRole).toLongLong(), 21);
QCOMPARE(model.data(index2, FlameGraphModel::DurationRole).toLongLong(), 12); QCOMPARE(model.data(index2, FlameGraphModel::DurationRole).toLongLong(), 13);
QCOMPARE(model.data(index, FlameGraphModel::CallCountRole).toInt(), 1); QCOMPARE(model.data(index, FlameGraphModel::CallCountRole).toInt(), 1);
QCOMPARE(model.data(index2, FlameGraphModel::CallCountRole).toInt(), 1); QCOMPARE(model.data(index2, FlameGraphModel::CallCountRole).toInt(), 1);
QCOMPARE(model.data(index, FlameGraphModel::DetailsRole).toString(), QCOMPARE(model.data(index, FlameGraphModel::DetailsRole).toString(),
@@ -154,10 +161,14 @@ void FlameGraphModelTest::testData()
QCOMPARE(model.data(index2, FlameGraphModel::LineRole).toInt(), 4); QCOMPARE(model.data(index2, FlameGraphModel::LineRole).toInt(), 4);
QCOMPARE(model.data(index, FlameGraphModel::ColumnRole).toInt(), 20); QCOMPARE(model.data(index, FlameGraphModel::ColumnRole).toInt(), 20);
QCOMPARE(model.data(index2, FlameGraphModel::ColumnRole).toInt(), 16); QCOMPARE(model.data(index2, FlameGraphModel::ColumnRole).toInt(), 16);
QCOMPARE(model.data(index, FlameGraphModel::NoteRole).toString(), QString("dings"));
// The flame graph model does not know that one of the notes belongs to row 2, and the
// other one to row 3. It will show both of them for all indexes with that type ID.
QCOMPARE(model.data(index, FlameGraphModel::NoteRole).toString(), QString("dings\nweg"));
QCOMPARE(model.data(index2, FlameGraphModel::NoteRole).toString(), QString()); QCOMPARE(model.data(index2, FlameGraphModel::NoteRole).toString(), QString());
QCOMPARE(model.data(index, FlameGraphModel::TimePerCallRole).toLongLong(), 20); QCOMPARE(model.data(index, FlameGraphModel::TimePerCallRole).toLongLong(), 21);
QCOMPARE(model.data(index2, FlameGraphModel::TimePerCallRole).toLongLong(), 12); QCOMPARE(model.data(index2, FlameGraphModel::TimePerCallRole).toLongLong(), 13);
QCOMPARE(model.data(index, FlameGraphModel::RangeTypeRole).toInt(), QCOMPARE(model.data(index, FlameGraphModel::RangeTypeRole).toInt(),
static_cast<int>(Javascript)); static_cast<int>(Javascript));
QCOMPARE(model.data(index2, FlameGraphModel::RangeTypeRole).toInt(), QCOMPARE(model.data(index2, FlameGraphModel::RangeTypeRole).toInt(),
@@ -170,12 +181,12 @@ void FlameGraphModelTest::testData()
QVERIFY(!model.data(index2, -10).isValid()); QVERIFY(!model.data(index2, -10).isValid());
QVERIFY(!model.data(QModelIndex(), FlameGraphModel::LineRole).isValid()); QVERIFY(!model.data(QModelIndex(), FlameGraphModel::LineRole).isValid());
for (int i = 1; i < 8; ++i) { for (int i = 1; i < 9; ++i) {
index = model.index(0, 0, index); index = model.index(0, 0, index);
QCOMPARE(model.data(index, FlameGraphModel::TypeRole).toString(), QCOMPARE(model.data(index, FlameGraphModel::TypeRole).toString(),
typeRoles[i % typeRoles.length()]); typeRoles[i % typeRoles.length()]);
QCOMPARE(model.data(index, FlameGraphModel::NoteRole).toString(), QCOMPARE(model.data(index, FlameGraphModel::NoteRole).toString(),
(i % typeRoles.length() == 0) ? QString("dings") : QString()); (i % typeRoles.length() == 0) ? QString("dings\nweg") : QString());
} }
QCOMPARE(model.data(index, FlameGraphModel::CallCountRole).toInt(), 1); QCOMPARE(model.data(index, FlameGraphModel::CallCountRole).toInt(), 1);
@@ -183,7 +194,7 @@ void FlameGraphModelTest::testData()
QCOMPARE(model.data(index2, FlameGraphModel::TypeRole).toString(), QCOMPARE(model.data(index2, FlameGraphModel::TypeRole).toString(),
FlameGraphModel::tr("Compile")); FlameGraphModel::tr("Compile"));
QCOMPARE(model.data(index2, FlameGraphModel::NoteRole).toString(), QString()); QCOMPARE(model.data(index2, FlameGraphModel::NoteRole).toString(), QString());
QCOMPARE(model.data(index2, FlameGraphModel::CallCountRole).toInt(), 2); QCOMPARE(model.data(index2, FlameGraphModel::CallCountRole).toInt(), 1);
} }
void FlameGraphModelTest::testRoleNames() void FlameGraphModelTest::testRoleNames()
@@ -206,7 +217,10 @@ void FlameGraphModelTest::testNotes()
{ {
manager.notesModel()->add(rangeModelId, 1, QString("blubb")); manager.notesModel()->add(rangeModelId, 1, QString("blubb"));
QCOMPARE(model.data(model.index(0, 0), FlameGraphModel::NoteRole).toString(), QCOMPARE(model.data(model.index(0, 0), FlameGraphModel::NoteRole).toString(),
QString("dings\nblubb")); QString("dings\nweg\nblubb"));
manager.notesModel()->remove(0);
QCOMPARE(model.data(model.index(0, 0), FlameGraphModel::NoteRole).toString(),
QString("weg\nblubb"));
manager.notesModel()->remove(0); manager.notesModel()->remove(0);
QCOMPARE(model.data(model.index(0, 0), FlameGraphModel::NoteRole).toString(), QCOMPARE(model.data(model.index(0, 0), FlameGraphModel::NoteRole).toString(),
QString("blubb")); QString("blubb"));