QTableWidget Update - slow

[DPinLV]

Hello,

I am using a QTableWidget to display 11 columns of data that I receive from a socket.
I receive about 200 lines of data in under 2 seconds. Each line uses a thread to make the signal/slot call for the GUI to display the data. In the GUI class, a new row is created and the line of data is parsed into the 11 different columns on the display. For some reason to update the table with all 200 lines of data, it takes close to 20 seconds. I have time print-outs around the 11 column entries, and it looks like the call to 'setItem' is the bottleneck.
Here's an example of one of the column entries:

start2 = QTime::currentTime ();

tbl_item = new QTableWidgetItem (QTableWidgetItem (QString (value_char)));
the_table->setItem ( location_index, column, tbl_item );

end2 = QTime::currentTime ();
elsp = (end2.second () - start2.second ());
if (elsp > 0)
logMessage (PRIORITY_DEBUG, "Elapsed flop Secs %u.%.3u",
elsp,
(end2.msec () - start2.msec ()));


I'm using a QTableWidget to take advantage of its search capabilities so I can make sure I am not adding duplicate data to the Table and I can update existing entries.
How I call the search:

QList<QTableWidgetItem*> results_list = the_table->findItems (QString::number(game_id), Qt::MatchExactly);

Should I use QTableView instead? Is it faster to add rows into the table?
What about QTableView's search capabilities, are they better than the search for QTableWidget? Does the Model/View architecture help with this?

Thanks,
DP

[jacek]

What about QTableView's search capabilities, are they better than the search for QTableWidget?
You can make them faster or even implement your model in such way, that they won't be needed just to check whether you should add the data or not.

http://www.qtcentre.org/forum/f-qt-programming-2/t-speed-of-setdata-lots-of-items-in-treeview-2888.html

[DPinLV]

Jacek,

I switch my table to QTableView and I created a custom Model derived from QAbstractTableModel. Unfortunately, the display is slower than before, I must have done something wrong.
In the class I have a vector of a specfic class that contains a vector of objects (a struct with a char string and its length).
When I add new rows I put the data into the vector

bool MyGameModel::insertRow(TabbedDataParser* tab_data_parser, const QModelIndex &index)
{
bool result = false;

if (0 != tab_data_parser)
{
TabbedDataParser* new_data = new TabbedDataParser ();
int position = m_data_container.size ();

position = index.column ();
*new_data = *tab_data_parser;
m_data_container.push_back (new_data);
result = true;
}

return result;
}

When the data is retrieved, I use the index.row to get the data from the vector, and then use my specialized obhject, to get the column data. These seems straight forward, yet I'm not gettig any improvements.

QVariant MyGameModel::data(const QModelIndex& index, int role) const
{
QVariant data;

if (index.isValid())
{
///EVEN DOING THIS IS SLOW
data = QString ("Well? %1").arg (index.column ());
return data;

if (Qt::DisplayRole == role)
{
// Get the data item
int container_size = m_data_container.size ();
if ((index.row () < container_size) && (index.column () < m_column_count))
{
TabbedDataParser* cached_data = (TabbedDataParser*)m_data_container[index.row ()];
if (0 != cached_data)
{
char* value_char = 0;
int column = index.column ();
GameUpdateIndices data_index = UPDATE_INDEX_UNKNOWN;

switch (column)
{
case GAME_ID_COLUMN:
data_index = ID;
break;
case GAME_NAME_COLUMN:
data_index = NAME;
break;
case GAME_WAITING_COLUMN:
data_index = PLAYERS_WAITING;
break;
default:
break;
}

if ((UPDATE_INDEX_UNKNOWN != data_index) && (cached_data->getDataValue (data_index, value_char)))
{
data = value_char;
}
}
}
}
}

return data;
}

I also overrode the methods for the column header data which is a QStringList of data.

QVariant MyGameModel::headerData (int section, Qt::Orientation orientation, int role) const
{
QVariant result;

if (section < m_header_strings.count ())
{
result = m_header_strings [section];
}

return result;
}

bool MyGameModel::setHeaderData (int section, Qt::Orientation orientation, const QVariant& value, int role)
{
m_header_strings = (m_header_strings << value.toString ());

return true;
}


I attach the model to TableView as such:

ui.m_Table->setModel (m_theModel);

And data is added like this:

m_theModel->insertRow ((TabbedDataParser*)&parsed_data);

Do you see anything I missed?

[jacek]

When I add new rows I put the data into the vector
What is the type of this vector? std::vector? AFAIR STL is awfully slow under windows (but I don't remember whose implementation).

You don't invoke beginInsertRows() and endInsertRow() in MyGameModel::insertRow().

[DPinLV]

I use a QVector on Windows XP

QVector<TabbedDataParser*> m_data_container;

Sorry, I had removed the begin/end insert calls to see if they were necessary in insertRow vs. insertRows. I have added them back in and there is still no improvement.

[wysota]

You don't invoke beginInsertRows() and endInsertRow() in MyGameModel::insertRow().

This shouldn't influence the speed so much.

My guess is, apart from anything else, that the other thread may be starving the model. Is this other thread really necessary? Or maybe you could add data by blocks and not one by one? Hundred messages per second need to be inserted into the queue and retrieved from it and processed, that's much work. Maybe you could use something like this:


QMutex mutex;
QList<TabbedDataParser*> sharedlist;

// "worker" thread:
mutex.lock();
bool needSignal = sharedlist.isEmpty();
sharedlist << new TabbedDataParser(...);
if(needSignal) emit newData();
mutex.unlock();

// "gui" thread
void GUIThreadClass::slotConnectedToNewData(){
mutex.lock();
while(!sharedlist.isEmpty()){
TabbedDataParser *elem = sharedlist.front();
sharedlist.pop_front();
processData(elem);
}
mutex.unlock();
}

You can still improve it. The code I wrote blocks the worker thread while the main thread processes data. You can make it that the list is only locked by the main thread when it actually retrieves data from it and when it processes the data, the other thread could still insert new items. Something like this could work:

void GUIThreadClass::slotConnectedToNewData(){
forever{
mutex.lock();
if(sharedlist.isEmpty()){
mutex.unlock(); break;
}
TabbedDataParser *elem = sharedlist.front();
sharedlist.pop_front();
mutex.unlock();
processData(elem);
}
}

This way instead of emitting 200 signals (which end up in the event queue), you emit one or two.

[DPinLV]

wysota,

I think I am doing something similar to what you suggest.
I receive data from a socket and I call a method that put data into a queue for retrieval by my worker thread.

void TableHandlingThread::addParsedDataQueue (int selected_view, const char* message, int message_length)
{
if ((0 != message) && (message_length > 0))
{
// Add to a vector
SignalDataContainerClass* signal_data = new SignalDataContainerClass ();

signal_data->init (selected_view, message, message_length, 0);
m_queue_mutex.lock ();
m_data_container_queue.insert (m_data_container_queue.begin (), signal_data);
m_wait_trigger.wakeAll ();
m_queue_mutex.unlock ();
}
}


The worker thread gets the trigger, drains the queue, and passes the message onto the gui thread for display. Where, m_data_container_queue, is a std::vector...

void TableHandlingThread::run ()
{
// Get the data from the queue and display it.
bool stop_thread = false;
int selected_view = 0;

// Set up the signal and slot for the data container
qRegisterMetaType< SignalDataContainerClass >( "SignalDataContainerClass" );
QObject::connect(m_emitter_class,
SIGNAL(dataContainerSignal (const SignalDataContainerClass& )),
m_pkr_receiver_class,
SLOT(receiveContainerDataSlot (const SignalDataContainerClass& )),
Qt::QueuedConnection);

QObject::connect(m_emitter_class,
SIGNAL(removeGameSignal (int , int )),
m_pkr_receiver_class,
SLOT(sendRemoveGameIDDataSender (int, int)),
Qt::QueuedConnection);

while (true)
{
SignalDataContainerClass* signal_data;

m_queue_mutex.lock ();
if (!m_data_container_queue.size ())
{
m_wait_trigger.wait (&m_queue_mutex);
}

// Get the stop variable
stop_thread = m_shutdown;
if (!stop_thread)
{
// Get item from queue
signal_data = m_data_container_queue.back ();
m_data_container_queue.pop_back ();
}
m_queue_mutex.unlock ();

if (stop_thread)
{
break;
}

if ((0 != signal_data) && (0 != m_emitter_class))
{
if (signal_data->m_message_length > 0)
{
m_emitter_class->sendDataContainerForSignal (*signal_data);
}
delete signal_data;
}
}
}
...
void TableHandlingThreadEmitter::sendDataContainerForSi gnal (const SignalDataContainerClass& table_data)
{
emit dataContainerSignal (table_data);
}

Does that accomplish the same as you suggest?:confused:

[jacek]

This shouldn't influence the speed so much.
Where did I write that it has any influence on the speed?

[DPinLV]

Also, the slot method, receiveContainerDataSlot, is where the TabbedDataParser object is created and the insertRow call is made.

[DPinLV]

What is the type of this vector? std::vector? AFAIR STL is awfully slow under windows (but I don't remember whose implementation).

You don't invoke beginInsertRows() and endInsertRow() in MyGameModel::insertRow().

jacek,
Are these calls necessary for insertRow and insertRows?

[jacek]

Are these calls necessary for insertRow and insertRows?
Yes, you need them to inform the view that model was updated.

[wysota]

Does that accomplish the same as you suggest?:confused:

No, you still emit a queued signal for every item. Each signal gets queued in the event queue and is processed separately later on. Either don't use that signal/slot mechanism at all here (if all that thread does is to wait for data and then emit it back into the queue, then you can safely skip it and do everything in a single thread without using the wait condition (you can do it in the slot after receiving data from network)) or limit the number of signals emitted.

And don't insert items one at a time into the model. And if you insert a group of items into the model at a time, either insert them at once (so that beginInsertRows and endInsertRows is called only once) or disable update of all views connected to the model (but the former should be easier to achieve).


Where did I write that it has any influence on the speed?
I'm sure you did! You must have changed your post afterwards! Damn you administrators! ;)

And more seriously, where did I write that you had written it had any influence on speed (remember "reported speech" -- "what has you done wiz your teacher!")?

[DPinLV]

And don't insert items one at a time into the model. And if you insert a group of items into the model at a time, either insert them at once (so that beginInsertRows and endInsertRows is called only once) or disable update of all views connected to the model (but the former should be easier to achieve).


I'm not sure there is any way to get around inserting the messages unless I sleep on the queue that drains them??? The idea is to display messages as they arrive on the socket.
In any case, I thought you could not invoke widget methods from non GUI threads?

[wysota]

But you can do network operations in the gui thread. And don't try to insert 200 entries a second one by one, your views will starve your app as they'll try to refresh themselves after every endInsertRows().

The solution I proposed before makes use of the fact that often you can process entries slower that they are generated, which enlarges the queue, causes more and more delays and causes a lock when you run out of some resource (for example buffer space). This is of course a problem, but you can change it to an asset if you add entries to the buffer as usual, but remove them not one by one, but all at once. This way you free the resource (queue) and can process entries "as fast as possible" -- remember there is no "immediately" in computer world. The user won't notice the difference if an entry is processed now or 10us later, as long as the order of items is preserved.

Each signal emition makes an overhead, each mutex lock makes an overhead. Each context change makes an overhead. Try to avoid them.

[DPinLV]

I'm not sure that applies to my model.
My socket operations are handled by an in-house developed library that uses asynchronous socket operations and its own thread pool (all strictly C++ no QT).

I assume this is why the GUI does not update when I make calls from the socket receive callback method. Then, I used the slot/signal approach so that when the socket callback received data it put in a queue that a QThread drained and sent to the slot on the GUI class. I see what you mean that if I call this 200 times, there are 200 emits and therefor 200 gui drawing emits as well. But, I feel that in my situation there is no other way since my socket stuff is not apart of the GUI thread.

Another thing I noticed is am I overriding the QAbstractItemModel methods correctly? I wasn't sure how to get my data into the model so my insertRow method is as such:

bool insertRow(TabbedDataParser* tab_data_parser, const QModelIndex &index = QModelIndex())

even though the signature of the QAbstrctItemModel class is

bool QAbstractItbool QAbstractItemModel::insertRow ( int row, const QModelIndex & parent = QModelIndex() )

Is that an issue?

[wysota]

My socket operations are handled by an in-house developed library that uses asynchronous socket operations and its own thread pool (all strictly C++ no QT).
That makes it more complicated...


But, I feel that in my situation there is no other way since my socket stuff is not apart of the GUI thread.
Of course there is... don't insert one row at a time :)


Another thing I noticed is am I overriding the QAbstractItemModel methods correctly? I wasn't sure how to get my data into the model so my insertRow method is as such:

bool insertRow(TabbedDataParser* tab_data_parser, const QModelIndex &index = QModelIndex())

even though the signature of the QAbstrctItemModel class is

bool QAbstractItbool QAbstractItemModel::insertRow ( int row, const QModelIndex & parent = QModelIndex() )
You are not overriding the insertRow method, because it has a different signature. But that's ok, you can have your own methods for manipulating the model. But I still suggest you insert a whole bunch of items at one go (using a custom method, of course).

[DPinLV]

wysota and jacek,

I have changed some things around and I am able to make multiple row updates where possible and it is definitely faster.

Thanks,
DP