PDA

View Full Version : QTableView performances



miraks
17th November 2008, 10:18
Hi,

I am using a QTableView to display a table containing 3949 lines and 12 columns.

It's very long (>1 min).

I am using my own model named SKGObjectModel (derived from SKGObjectModelBase (derived from QAbstractItemModel)).

Here is a part of my analysis:


##methode nb call time in this method average %
##SKGObjectModel::data 2682080 32612,7 0,01 38,43%
##SKGObjectModelBase::rowCount 2683753 12215,9 0 14,39%
##SKGObjectModelBase::index 2683717 11362,9 0 13,39%
##QAbstractItemModel::reset 11 10761 978,27 12,68%
##SKGObjectModelBase::data 2323211 9616,11 0 11,33%
##SKGObjectModelBase::headerData 244223 3669,73 0,02 4,32%
##SKGServices::executeSelectSqliteOrder(QSqlDataba se) 114 1544,71 13,55 1,82%
##SKGServices::executeSqliteOrder(QSqlDatabase) 287 1207,02 4,21 1,42%
##SKGMainPanel::SKGMainPanel 1 559,59 559,59 0,66%
##SKGMainPanel::setNewTabContent 1 429,53 429,53 0,51%
##SKGObjectBase::getObjects 37 383,36 10,36 0,45%

As you can see, data rowCount and index are very performant (average ~ 0) but I have bad performances because of these methods are called to many times (~ 2 600 000 times).

Do you know why ?
What can I do to avoid this ?

In advance, thank you for your help.

PS: the code is there: http://skrooge.svn.sourceforge.net/viewvc/skrooge/

wysota
17th November 2008, 13:09
Don't call reset - you'll reduce the number of calls 11 times :) Also, what does ::data() implementation look like?

miraks
17th November 2008, 13:52
Hi and thank you for your answer,

In fact reset is called only 3 times for this table:
1-when the table is initialized and empty
2-when the table is refreshed to display only 20 lines
3-when the table is refreshed to display all lines (this is the case with bad performances.

All other calls are for other tables containing around 10 lines.

So, I can't reduce the number of call to reset.

Here is the code:


QVariant SKGObjectModelBase::data(const QModelIndex &index, int role) const
{
if (!index.isValid()) return QVariant();
_SKGTRACEIN(10, "SKGObjectModelBase::data");

SKGObjectBase* obj = (SKGObjectBase*) index.internalPointer();

switch ( role ) {
case Qt::DisplayRole:
case Qt::EditRole:
case Qt::UserRole: {
QString att=listAttibutes[index.column()];
QString val=obj->getAttribute(att);

if (att.startsWith("f_")) {
double dval=SKGServices::stringToDouble(val);
return dval;
} else if (att.startsWith("i_")) {
return SKGServices::stringToInt(val);
} else if (att.startsWith("d_")) {
QDate dval=SKGServices::stringToTime(val).date();
if (role == Qt::DisplayRole) {
return KGlobal::locale()->formatDate (dval, KLocale::FancyShortDate);
} else {
return dval;
}
} else if (getRealTable()=="doctransaction" && att=="t_savestep") return "";
return val;
}

case Qt::TextColorRole: {
//Text color
QString att=listAttibutes[index.column()];
if (att.startsWith("f_")) {
QVariant value_displayed = SKGObjectModelBase::data(index, Qt::UserRole);
bool ok=false;
double value_double=value_displayed.toDouble(&ok);
if (ok && value_double<0) return qVariantFromValue(QColor(Qt::red));
}
break;
}
case Qt::TextAlignmentRole: {
//Text alignment
QString att=listAttibutes[index.column()];
return (int)(Qt::AlignVCenter|(att.startsWith("f_") || att.startsWith("i_") ? Qt::AlignRight : Qt::AlignLeft));
}
case Qt::DecorationRole: {
//Decoration
if (index.column() == 0 && getRealTable()=="node") {
SKGNodeObject node =*obj;
QStringList data=SKGServices::splitCSVLine(node.getData());
if (data.count()>2) return qVariantFromValue((QIcon) KIcon(data.at(2)));
else {
return qVariantFromValue((QIcon) KIcon(node.getName()==i18n("autostart") ? "folder-bookmarks" : "folder"));
}
} else if (index.column() == 0 && getRealTable()=="doctransaction") {
return qVariantFromValue((QIcon) KIcon(obj->getAttribute("t_mode")=="U" ? "edit-undo" :"edit-redo"));
} else if (getRealTable()=="doctransaction" && listAttibutes[index.column()]=="t_savestep") {
if (obj->getAttribute("t_savestep")=="Y") return qVariantFromValue((QIcon) KIcon("document-save"));
}
break;
}
case Qt::ToolTipRole: {
//Tooltip
if (getRealTable()=="doctransaction") {
QStringList msg;
document->getMessages(obj->getID(), msg);
int nbMessages=msg.count();
if (nbMessages) {
QString message;
for (int i=0; i<nbMessages; ++i) {
if (i!=0) message+='\n';
message+=msg.at(i);
}

return message;
}
}
break;
}
default : {
}
}

return QVariant();
}

miraks
17th November 2008, 13:53
Here is the code for SKGObjectModel::data:

QVariant SKGObjectModel::data(const QModelIndex &index, int role) const
{
if (!index.isValid()) return QVariant();
_SKGTRACEIN(10, "SKGObjectModel::data");

SKGObjectBase* obj = (SKGObjectBase*) index.internalPointer();
switch ( role ) {
case Qt::DisplayRole:
case Qt::EditRole:
case Qt::UserRole: {
QString att=listAttibutes[index.column()];
QString val=obj->getAttribute(att);
if (((getRealTable()=="operation" || getRealTable()=="recurrentoperation") &&
(att=="t_bookmarked" || att=="t_status")) ||
(getRealTable()=="account" && att=="t_close")) {
if (role==Qt::UserRole) return val;
else return "";
} else if (getRealTable()=="account" && att=="t_type") {
if (val=="C") return i18n("Current");
else if (val=="D") return i18n("Credit card");
else if (val=="A") return i18n("Actif");
else if (val=="I") return i18n("Investment");
else return i18n("Other");
} else if (getRealTable()=="unit" && att=="t_type") {
if (val=="1") return i18n("Primary currency");
else if (val=="2") return i18n("Secondary currency");
else if (val=="C") return i18n("Currency");
else if (val=="S") return i18n("Stock");
else return i18n("Object");
} else if (att.startsWith("f_")) {
double dval=SKGServices::stringToDouble(val);
if (role == Qt::DisplayRole) {
QString unit=" ";
if (QString::compare(att, "f_QUANTITY", Qt::CaseInsensitive)!=0 && QString::compare(att, "f_REALQUANTITY", Qt::CaseInsensitive)!=0) {
unit=((SKGDocumentBank*) getDocument())->getPrimaryUnit();
if (getRealTable()=="unitvalue" && !cacheUnit.isEmpty()) {
unit=cacheUnit;
}
}
return KGlobal::locale()->formatMoney (dval, unit, 2);
} else {
return dval;
}
} else if (att.startsWith("i_")) {
if (att=="i_number" && val=="0") return "";
return SKGServices::stringToInt(val);
} else if (att.startsWith("d_")) {
QDate dval=SKGServices::stringToTime(val).date();
if (role == Qt::DisplayRole) {
return KGlobal::locale()->formatDate (dval, KLocale::FancyShortDate);
} else {
return dval;
}
}
return val;
}
case Qt::DecorationRole: {
//decoration
if (index.column() == 1 && getRealTable()=="account") {
QString t_icon=obj->getAttribute("t_icon");
if (t_icon.isEmpty()) t_icon=obj->getAttribute("t_ICON");
if (!t_icon.isEmpty()) {
QDir dirLogo(KStandardDirs::locate("data", QString::fromLatin1("skrooge/images/logo/")+KGlobal::locale()->language()+'/'));
return qVariantFromValue(QIcon(dirLogo.absoluteFilePath(t _icon)));
}
} else if (index.column() == 0 && getRealTable()=="category") {
QString t_TYPEEXPENSE=obj->getAttribute("t_TYPEEXPENSE");
if (t_TYPEEXPENSE==tr("Expense")) return KIcon("skrooge_category_expense");
else if (t_TYPEEXPENSE==tr("Income")) return KIcon("skrooge_category_income");
return KIcon("skrooge_category");
} else {
QString att=listAttibutes[index.column()];

if (getRealTable()=="operation" || getRealTable()=="recurrentoperation") {
if (att=="t_mode") {
if (obj->getAttribute("i_group_id")!="0") return KIcon("skrooge_transfer");
} else if (att=="t_CATEGORY") {
if (SKGServices::stringToInt(obj->getAttribute("i_NBSUBCATEGORY"))>1) return KIcon("skrooge_split");
} else if (att=="t_comment" && getRealTable()=="operation") {
if (obj->getAttribute("i_NBRECURRENT")!="0") return KIcon("skrooge_schedule_origin");
else if (obj->getAttribute("r_recurrentoperation_id")!="0") return KIcon("chronometer");
} else if (att=="t_bookmarked" && obj->getAttribute("t_bookmarked")=="Y") return KIcon("rating");
}
}
break;
}
case Qt::TextColorRole: {
//Text color
QString att=listAttibutes[index.column()];
if (getRealTable()=="recurrentoperation" &&
att=="d_DUEDATE" &&
SKGServices::stringToTime(obj->getAttribute("d_DUEDATE")).date()<=QDate::currentDate())
return qVariantFromValue(QColor(Qt::red));
break;
}
case Qt::CheckStateRole: {
//CheckState
QString att=listAttibutes[index.column()];
if (getRealTable()=="operation" && att=="t_status") {
QString t_status=obj->getAttribute("t_status");
if (t_status=="P") return Qt::PartiallyChecked;
else if (t_status=="C") return Qt::Checked;
else if (t_status=="N") return Qt::Unchecked;
} else if (getRealTable()=="account" && att=="t_close") {
QString t_status=obj->getAttribute("t_close");
if (t_status=="Y") return Qt::Checked;
else return Qt::Unchecked;
} else if (getRealTable()=="recurrentoperation" && att=="i_auto_write_days") {
QString t_auto_write=obj->getAttribute("t_auto_write");
if (t_auto_write=="Y") return Qt::Checked;
else return Qt::Unchecked;
} else if (getRealTable()=="recurrentoperation" && att=="i_warn_days") {
QString t_auto_write=obj->getAttribute("t_warn");
if (t_auto_write=="Y") return Qt::Checked;
else return Qt::Unchecked;
}
break;
}
case Qt::ToolTipRole: {
//Tooltip
QString secondaryUnit=((SKGDocumentBank*) getDocument())->getSecondaryUnit();
if (!secondaryUnit.isEmpty()) {
QString att=listAttibutes[index.column()];
if (att.startsWith("f_") && att!="f_QUANTITY" && att!="f_REALQUANTITY") {
double unitval=((SKGDocumentBank*) getDocument())->getSecondaryUnitValue();
if (unitval) {
double dval=SKGServices::stringToDouble(obj->getAttribute(att))/unitval;
return KGlobal::locale()->formatMoney (dval, secondaryUnit, 2);
}
}
}
break;
}
default: {
}
}
return SKGObjectModelBase::data(index, role);
}

wysota
17th November 2008, 14:38
In fact reset is called only 3 times for this table:
So reduce it to 0. There is a 99% chance you don't need it.


So, I can't reduce the number of call to reset.
Sure you can. You can remove all rows and insert new ones, if you have to. It'll be much faster than calling reset. And you don't have to reset an empty model.

Your data() implementations are very complicated, try simplifying them. Here are some hints:
- don't compare strings, compare enums, single characters, ints or hashes from strings
- cache, cache, cache
* don't recompute data, cache it - there is a chance it'll be needed soon
* if you compute data for the item, compute not only the display role, but all roles - the view will query for more than one role for each index, why repeat calculations?

miraks
24th November 2008, 11:41
Hi,

I replaced all calls to reset by something based on beginRemoveRows + endRemoveRows + beginInsertRows + endInsertRows.

But I still have the same performances (bad performances :( ) and the same number of calls on data method:

##methode nb call millisecondes moyenne min max propre moyenne propre %
##SKGObjectModel::data 2706598 46342,4 0,02 0 64,25 35899,5 0,01 33,75%
##QAbstractItemModel::refresh-fill 14 48237 3445,5 0,09 47583,1 19442,2 1388,73 18,28%
##SKGObjectModelBase::rowCount 2709500 15354,5 0,01 0 53,88 15354,5 0,01 14,44%
##SKGObjectModelBase::index 2709435 13351,1 0 0 51,89 13351,1 0 12,55%
##SKGObjectModelBase::data 2353264 11256,9 0 0 64,2 10569,1 0 9,94%

wysota
25th November 2008, 07:56
Did you reimplement data() the way I suggested?

calmspeaker
26th November 2008, 01:54
Do not use reset() as you could,because reset() will clear the state of the items in the table, for example, the selected state. And the beginInsertRows(), endInsertRows() will reduce the efficiency. You could emit the signals layoutAboutToBeChanged()before you insert warnings,layoutChanged() after you insert warnings.

The tableview could be very efficent, I have tried to display 100,000 items, and there is no obvious delay.

er, I have a question, what tool do u use to test your codes' efficency?:)

miraks
26th November 2008, 10:33
Did you reimplement data() the way I suggested?
Not yet.

Do not use reset() as you could,because reset() will clear the state of the items in the table, for example, the selected state. And the beginInsertRows(), endInsertRows() will reduce the efficiency. You could emit the signals layoutAboutToBeChanged()before you insert warnings,layoutChanged() after you insert warnings.
In any case, in my scenario, I have to fill the table with around 3400 new lines.
At the beginning the table contains 20 lines.
After a user action, the table must contain around 3400 new lines.
I will try with layoutAboutToBeChanged() and layoutChanged().

The tableview could be very efficient, I have tried to display 100,000 items, and there is no obvious delay.
Did you tried with decorations, colors, alignment ?
I noticed bad performances just by adding background color.

what tool do u use to test your codes' efficency?
My own component.
In fact, my traces system is able (with some build directives activated) to compute the time consumed by a part of code (a method for example).
This system is less time consuming that something like Callgrind.

calmspeaker
27th November 2008, 02:32
In any case, in my scenario, I have to fill the table with around 3400 new lines.
At the beginning the table contains 20 lines.
After a user action, the table must contain around 3400 new lines.

I suggest you only call the pair signal(beginlayoutchanged, layoutchanged) once, in your situation, that is a way to improve the efficiency.


Did you tried with decorations, colors, alignment ?
I noticed bad performances just by adding background color.
I only tried to changed all the items' background color every per second.The efficiency is acceptable.



My own component.
In fact, my traces system is able (with some build directives activated) to compute the time consumed by a part of code (a method for example).
This system is less time consuming that something like Callgrind.
Could it be shared?;)

wysota
27th November 2008, 10:51
Have you tried gprof? It's faster than callgrind because it doesn't emulate the cpu and it's standard, so there are tools to use it. You can use it with GCC only, but I take it that's not a problem for you :)

miraks
28th November 2008, 19:07
Hi calmspeaker,

I prepared for you a package containing:
1-A class managing errors with history.
2-A class managing indented traces with error and profiling.
3-A document to explain how to use it.

I hope it will help you.
If you need help, contact me.

I will test your proposal.

miraks
28th November 2008, 19:36
Hi,

Here is the result of my test with layoutAboutToBeChanged + layoutChanged.

Without this modification, I get:

##methode nb call millisecondes moyenne min max propre moyenne propre %
##SKGObjectModel::data 2705890 43950,9 0,02 0 60,99 31727,7 0,01 39,24%
##SKGObjectModelBase::data 2347017 12969,1 0,01 0 37,98 12309,6 0,01 15,23%
##SKGObjectModelBase::rowCount 2707786 10864,1 0 0 22,39 10864,1 0 13,44%
##SKGObjectModelBase::index 2707746 9032,57 0 0 27,68 9032,57 0 11,17%
##QAbstractItemModel::reset-fill 14 21129,6 1509,26 0,19 20674,1 8237,73 588,41 10,19%
##SKGObjectModelBase::headerData 251522 4541,63 0,02 0 16,52 3054,3 0,01 3,78%
##SKGServices::executeSelectSqliteOrder(QSqlDataba se) 174 2351,37 13,51 0,03 730,59 2351,37 13,51 2,91%
##SKGMainPanel::SKGMainPanel 1 4144,91 4144,91 4144,91 4144,91 820,56 820,56 1,01%

With the modification:

##methode nb call millisecondes moyenne min max propre moyenne propre %
##SKGObjectModel::data 1939034 31138,4 0,02 0 32,87 22583,8 0,01 37,16%
##SKGObjectModelBase::data 1667650 9058,82 0,01 0 27,76 8592,84 0,01 14,14%
##QAbstractItemModel::reset-fill 14 21189,3 1513,52 0,13 20886,5 8319,58 594,26 13,69%
##SKGObjectModelBase::rowCount 1941008 7398,6 0 0 35,16 7398,6 0 12,17%
##SKGObjectModelBase::index 1940968 6369,83 0 0 20,05 6369,83 0 10,48%
##SKGServices::executeSelectSqliteOrder(QSqlDataba se) 174 2528,14 14,53 0,03 759,23 2528,14 14,53 4,16%
##SKGObjectModelBase::headerData 170665 2962,19 0,02 0 12,09 1976,83 0,01 3,25%
##SKGServices::executeSqliteOrder(QSqlDatabase) 379 1612,22 4,25 0,03 533,94 831,15 2,19 1,37%
##SKGMainPanel::SKGMainPanel 1 3931,13 3931,13 3931,13 3931,13 715,1 715,1 1,18%

Conclusion:
It's better because the number of call reduced. :)
It's not enough. :(

wysota
28th November 2008, 21:23
I really suggest to modify the implementation of data(). It is currently very inefficient and it's one of the most often called methods.

miraks
28th November 2008, 21:53
I really suggest to modify the implementation of data(). It is currently very inefficient and it's one of the most often called methods.

I will try but it's not so easy !
I will take you informed.

In any case, thank you for your help.

miraks
29th November 2008, 13:53
Hi wysota,

I have got 2 bad news:

1-I optimized data method and after that, I commented in data methods all the code concerning decoration, tooltip, alignment, etc to keep only the text. Even with this, I win only 6% of performances. :(.
Conclusion: even with perfect code in data, I won't have good performances.

2-The solution based on layoutAboutToBeChanged + layoutChanged generates some regressions. :(
(indexes seems to be not recomputed and so are still pointing on deleted internal data).

I don't know what to do.

PS1: I attached my new implementation.
PS2: I don't know if it's very important, but I use a proxy (filter & sort) too.

wysota
29th November 2008, 16:48
The implementation is still very inefficient. You are still comparing strings instead of enums or numbers and you don't do any serious caching of calculated data.

6% performance gain is already much, don't underestimate it.

miraks
30th November 2008, 11:29
Hi wysota,

Today, I have got a good new.
I understood why I have so bad performances.
In fact, it's due to these lines:

ui.kOperationView->verticalHeader()->setResizeMode(QHeaderView::ResizeToContents);
ui.kOperationView->horizontalHeader()->setResizeMode(QHeaderView::ResizeToContents);

If I comment these lines, I have got:

##methode , nb call , millisecondes , moyenne , min , max , propre , moyenne propre
##SKGServices::executeSelectSqliteOrder(QSqlDataba se) , 173 , 2250.3 , 13.0075 , 0.0319843 , 757.705 , 2250.3 , 13.0075
##SKGObjectModelBase::data , 77777 , 2022.72 , 0.0260067 , 0.00187886 , 17.3499 , 2011.33 , 0.0258602
##SKGObjectModelBase::refresh-reset , 13 , 4078.05 , 313.696 , 0.00701368 , 3962.67 , 1081.16 , 83.166
##SKGObjectModel::data , 71620 , 2738.77 , 0.0382403 , 0.00292969 , 17.3791 , 776.344 , 0.0108398
##SKGServices::executeSqliteOrder(QSqlDatabase) , 379 , 1319.9 , 3.48257 , 0.0279003 , 444.459 , 679.188 , 1.79205
##SKGMainPanel::SKGMainPanel , 1 , 2419.67 , 2419.67 , 2419.67 , 2419.67 , 372.141 , 372.141
##SKGObjectModelBase::index , 73266 , 295.21 , 0.00402929 , 0.00190628 , 10.6721 , 295.21 , 0.00402929
##SKGObjectModelBase::rowCount , 73305 , 223.636 , 0.00305076 , 0.00187886 , 11.12 , 223.636 , 0.00305076
##SKGObjectModelBase::clear , 17 , 201.88 , 11.8753 , 0.00792778 , 176.784 , 201.88 , 11.8753
##SKGObjectBase::getObjects , 62 , 1078.61 , 17.3969 , 0.0670469 , 932.943 , 184.245 , 2.9717
##SKGObjectModelBase::headerData , 9244 , 137.556 , 0.0148806 , 0.00188088 , 1.31897 , 104.604 , 0.0113158
##SKGSortFilterProxyModel::filterAcceptsRow , 4022 , 336.626 , 0.0836961 , 0.023957 , 0.784051 , 90.5983 , 0.0225257

As you can see, the number of calls is drastically reduced :) and performances are better (before 43s into data method, now 2s into data method ==> it's better than 6% ;) ).

Is it normal to have so bad performances when ResizeToContents is used ?

wysota
1st December 2008, 10:25
Yes, that's normal. sizeHint is queried all the time which in turns calls data() all the time, etc. I thought you knew that so I didn't even bother to mention it, sorry :o

But honestly it's because of your hand-crafted profiler - it doesn't monitor methods which are called automatically by Qt or that are implemented by Qt, hence no references to sizeHint reading and hence my impression you weren't resizing columns to contents.