QUdpSocket data loss using threads

[Chops211]

Hi all,
I have been using QUdpSocket for a while now and we have legacy code that reimplements QThread. I converted the code so that I have a QObject named UDPServer that is passed to a QThread and executes it’s event loop. Everything there works great. The problem I face is that the device I am communicating with bursts data packets, so I can a large chunk of UDP packets in a short period of time. I get significant data loss even with a large UDP buffer (using WinSock setsockopt). Here is what the architecture looks like:

QThread 1:
Running the event loop for the UDPServer which contains the QUDPSocket

QThread 2:
Running the event loop for FileWriter (Basically writes the data to a file when signalled from the UDPServer)

The UDPServer stores x number of data packets before signalling the FileWriter to write to a file. This is to decrease overhead and have burst writes. I basically toggle between two receive buffers and once one is full, send that off to be written.

Here are my questions:
In the UDPServer, the QUDPSocket readyReady signal is connected to slotReadReady(). In there we use the same code as the QUDOSocket example:

while( m_p_UDPSocket->hasPendingDatagrams() ) {
// Process the data
}

Is the while loop blocking the QUDPSocket from getting more data packets since it is in the same thread? Does anyone know of a good approach to a high performance QUDPSocket receiver?

Thank you for taking the time to read all of this, it’s a lot

---

Added after 1 29 minutes:

Update:
This implementation "the right way to do threads" by moving my UDP receiver to a QThread works BUT I see my non-paged memory pool increase until it overflows. The only way around this to have a forever loop around the while loop to read the pending datagrams. It seems as if the signals-slots implementation blocks the UDP socket from receiving datagrams from the OS socket buffer. Is there anyway around this? I don't think subclassing QThread is the answer since I need signals-slots to offload the read datagrams to the filewriter.

[jefftee]

As I am sure you are aware, UDP is a connectionless protocol and the does not guarantee the packets will arrive in the same sequence as sent or even arrive at all for that matter. If this is important to your application, you should use TCP or you will have to implement your own method of identifying UDP packets that were sent but not received (and handling sequencing if order is important). Switching to TCP would do that for you automatically and if you must stick with UDP, then you will have to implement a send/ack application method so that the sender is aware of which packets you have received and (after an appropriate time out period) resend packets that were not acknowledged, etc.

Qt's networking classes are all asynchronous, so what are you doing inside that while loop? Have you tried adding a QCoreApplication::processEvents() call inside that while loop? Remember, your event loop will not dispatch events until you have returned from that slot, unless you call processEvents(), etc.

The memory increase you are seeing is most certainly due to your application leaking memory, so you should debug to find the source of that leak.

Lastly, I would leave your design such that all networking is done in your main thread. You can use signals/slots to send received data over to your file writer thread w/o adding a lot of complexity. I would do a minimal amount of processing to just receive data from the socket and store in a buffer on readyRead(). When you determine the buffer is full or some predetermined amount of time has elapsed, emit a signal that sends the buffer contents to your file writer, etc.

Trying to add/subclass QThread to handle the networking will complicate your application and is usually not necessary.

Can you confirm what processing occurs inside your slot connected to readyRead() today? Specifically, what's inside your while loop that checks for hasPendingDatagrams()?

Thanks,

Jeff

[anda_skoa]

I don't think subclassing QThread is the answer since I need signals-slots to offload the read datagrams to the filewriter.

Just to be sure: sending signals will work with or without event loop, receiving cross-thread slot invocation requires an event loop.

You can of course also transfer data to the receiver directly using method calls with proper thread synchronisation.

Cheers,
_

[Chops211]

As I am sure you are aware, UDP is a connectionless protocol and the does not guarantee the packets will arrive in the same sequence as sent or even arrive at all for that matter. If this is important to your application, you should use TCP or you will have to implement your own method of identifying UDP packets that were sent but not received (and handling sequencing if order is important). Switching to TCP would do that for you automatically and if you must stick with UDP, then you will have to implement a send/ack application method so that the sender is aware of which packets you have received and (after an appropriate time out period) resend packets that were not acknowledged, etc.

The device I am connected to is strictly UDP and we have a custom acknowledge/timeout mechanism to support packets being dropped. Since the device is going to be connected to a server with direct ethernet access, packets will not be received out of turn. UDP was selected since we have cases of high data rates (> 60MB/s) and we plan on using the device over a lossy network. TCP would stall in the cases where the network is dropped, and the device does not have adequate memory to support such a large buffer

Qt's networking classes are all asynchronous, so what are you doing inside that while loop? Have you tried adding a QCoreApplication::processEvents() call inside that while loop? Remember, your event loop will not dispatch events until you have returned from that slot, unless you call processEvents(), etc.

I have not tried processEvents(), but I will. You are saying that the thread is blocking the handling of the UDP receive from the OS? I was wondering this as well and placed the UDP socket outside of the thread that is processing the datagrams. Simply connected the readReady() signal from the UDP socket to a slot in the thread. The non-paged memory pool still increases. It is as if the UDP socket is not reading from the OS level socket memory fast enough.

The memory increase you are seeing is most certainly due to your application leaking memory, so you should debug to find the source of that leak.

I originally thought this as well but at low data rates, the memory does not increase. As a matter of fact, when using a forever loop to process the datagrams as fast as possible, there is no memory increase. The CPU usage does go up 10% but I am able to achieve 70MB/s without any data loss. It's as if the signals-slots mechanism is too slow for high performance.

Lastly, I would leave your design such that all networking is done in your main thread. You can use signals/slots to send received data over to your file writer thread w/o adding a lot of complexity. I would do a minimal amount of processing to just receive data from the socket and store in a buffer on readyRead(). When you determine the buffer is full or some predetermined amount of time has elapsed, emit a signal that sends the buffer contents to your file writer, etc.

I set the MainWindow to be the parent of the QUdpSocket and did as you suggest. Nothing changed. I am storing the data between two buffers; when one is filled, I send it off to be processed, while filling up the next one. It's circular and works pretty well. I set the buffer size to be pretty large and since my computer has an SSD, the write speed to the file is fast and not causing any dropouts.

Trying to add/subclass QThread to handle the networking will complicate your application and is usually not necessary.

In the past, this was used since the original developer did not fully understand QThreads, but it seemed like the performance was better without the use of signals-slots. There is sufficient documentation that the signals-slots is around 10x slower than a callback function. It is hard to find the exact timing delay between the signal and slot. I would prefer not to subclass QThread.

Can you confirm what processing occurs inside your slot connected to readyRead() today? Specifically, what's inside your while loop that checks for hasPendingDatagrams()?

It simply reads the datagram, checks a 32-bit packet type in the datagram, and emits a signal depending on the packet type. If the packet type is a "data" packet type, I memcpy to a buffer as described above and send a signal when that buffer is full

Thanks,

Jeff

No, thank you for taking the time to help me

Just to be sure: sending signals will work with or without event loop, receiving cross-thread slot invocation requires an event loop.

You can of course also transfer data to the receiver directly using method calls with proper thread synchronisation.

Cheers,
_

There are some slots that I need in the thread which is why I do need an event loop. I have a worker QObject "UDPReceiver" that handles all the signals-slots and creates the QUdpSocket. None of the allocation is done in the constructor, but in a slot called slotStartUDPReceiver(), which is connected to the QThread started() signal. I made sure that the QUdpSocket is in the correct thread.

[anda_skoa]

There are some slots that I need in the thread which is why I do need an event loop.

Again, just to be sure: you only need an event loop for the receiver thread if you have slots connected to signals emitted by another thread.
Signal/slot connections within the same thread don't need an event loop.

I have a worker QObject "UDPReceiver" that handles all the signals-slots and creates the QUdpSocket.

Using an event driven object requires an event loop, but that is independent of signal/slot connections.

Cheers,
_

[Chops211]

Adding an event loop makes it work! I guess the event loop allows the socket to read from the OS buffer without blocking when in the while loop? I am able to get 55MB/s with this, and would ideally want to get up to 70MB/s.

I simply created an event loop within the thread and call m_p_eventLoop->exec();

Does that make sense?

[jefftee]

The default QThread::run() virtual method simply runs an event loop. Have you overridden the QThread::run() virtual method?

If you have, you should just ensure that your run() function execs the event loop. If you haven't overridden QThread::run(), I don't understand why you would need to create and exec() a separate event loop.

You have started your QThread, correct?

[Chops211]

I agree, I don't know why the event loop is necessary. All I want is for the UDP socket to continuously handle reads while I am reading from the socket. I start the thread as follows:

QThread *p_thread = new QThread();
m_p_UDPReceiver = new UDPReceiver();

m_p_UDPReceiver->moveToThread( p_thread );
.. Connect all signals and slots

p_thread->start( QThread::TimeCriticalPriority );


There are slots connected to signals that are emitted from a different thread. I don't understand why an eventloop causes the socket to read faster, if there is already an event loop being run by the thread. The QUdpSocket is in the same thread as the slot the readyRead() signal calls. There shouldn't be any issue, right? I've placed the QUdpSocket into the main thread but then I can't read from it in the receiver thread because socketnotifiers cannot be enabled/disabled from different threads. Reading the data off the socket in the main thread and then sending them to the receiver thread is an option, but then I definitely need an event loop because it will block all GUI events.

[jefftee]

That looks good to me. After the p_thread->start(), your QThread should be running an event loop via the default QThread::run() virtual method. Does your UDPReceiver class have any slots where you are using a while loop or any other long running tasks that would prevent the event loop from running?

Where in your UDPReceiver class did you add the m_p_eventLoop->exec()? Can you post the source and header for the UDPReceiver class?

[Chops211]

Yes, as per the QUdpSocket example, my UDPReceiver has the following:

// Read all available data packets
while( m_p_UDPReceiveSocket->hasPendingDatagrams() ) {
  // Read the datagram
  m_p_UDPReceiveSocket->readDatagram( m_p_datagram,
    m_p_UDPReceiveSocket->pendingDatagramSize() );
 
   /** Some small amount of work */
}

// Read all available data packets
while( m_p_UDPReceiveSocket->hasPendingDatagrams() ) {
  // Read the datagram
  m_p_UDPReceiveSocket->readDatagram( m_p_datagram,
    m_p_UDPReceiveSocket->pendingDatagramSize() );

   /** Some small amount of work */
}

To copy to clipboard, switch view to plain text mode 

I believe the while loop is blocking the UDP socket from handling events from the OS, which is why my non-paged memory pool increases up until the size of the UDP socket buffer.

P.S. i edited the previous post. This is what I wrote in case you missed it:
There are slots connected to signals that are emitted from a different thread. I don't understand why an eventloop causes the socket to read faster, if there is already an event loop being run by the thread. The QUdpSocket is in the same thread as the slot the readyRead() signal calls. There shouldn't be any issue, right? I've placed the QUdpSocket into the main thread but then I can't read from it in the receiver thread because socketnotifiers cannot be enabled/disabled from different threads. Reading the data off the socket in the main thread and then sending them to the receiver thread is an option, but then I definitely need an event loop because it will block all GUI events.

I really appreciate your help btw. Wish I could buy you a beer

UDPReceiver.cpp
UDPReceiver.h

Here are the files.

[jefftee]

I would be curious to see if a QCoreApplication::processEvents() (or QApplication depending on what you're using in main) as the last statement inside your while loop would work as opposed to creating and exec() your own event loop.

Could you give that a try and report results?

[Chops211]

I'll run that right now. Thanks

That makes it run much slower. I should be getting around 55MB/s and instead I get 16MB/s by adding the processEvents().

[jefftee]

Ok, worth a shot...

I am stumped why you need to create your own event loop and exec it since the default QThread::run() should be running an event loop. Perhaps others may be able to help, but at least it sounds like you have a method that is meeting your needs. If you're like me though, I hate it when I don't understand why something is working the way it is...

[Chops211]

Exactly. I really don't want to leave this unexplained. Also, it works but not to the performance I need. A forever loop that continuously polls the socket gives me a speed increase to 70MB/s. Ideally I can find a method using signals and slots that can achieve the same performance.

---

Added after 1 41 minutes:

Found something interesting: https://qt.gitorious.org/qt/qtbase/c...1395dfffc3ed18 Definitely works better when I upgraded to 5.4.1 but I still see the non-paged memory pool usage increase. I'm assuming I probably need to wait for 5.4.2 or I am doing something wrong.

[Chops211]

Good news is that adding an eventloop within the thread and simply just running exec() has resolved the issue that I am seeing. The bad news is that the new Qt5.4.2 update causes the non-paged memory pool to overflow with or without this fix. Whatever was patched in QUdpSocket in Qt5.4.2 has now broke the class.

[jefftee]

I haven't upgraded to 5.4.2 yet, but as soon as I do, I'll fire up a UDP client/server app that I have and let it smoke test to see if I also encounter the non-paged memory pool issue you're running into.

[Davide]

Adding an event loop makes it work! I guess the event loop allows the socket to read from the OS buffer without blocking when in the while loop? I am able to get 55MB/s with this, and would ideally want to get up to 70MB/s.

I simply created an event loop within the thread and call m_p_eventLoop->exec();

Does that make sense?

Hello Chops.
I'm having a similar issue you had and now I'm trying to solve it as you did.
May I please know where did you add the m_p_eventLoop->exec()?
Is m_p_eventLoop a QEventLoop object? Where/how did you declare it?
I could not find it in the code you shared (UDPReceiver.cpp and UDPReceiver.h).

Thanks in advance :-)

[Pereubu2018]

What is the meaning of while? Is that meaning that is going to work when the first packet is received and afterthat receiving is out of while loop. Then second arrives? But you are out of while loop...That works if packets get to the same time?

---

Added after 1 19 minutes:

I am not quite for this:
m_p_UDPReceiveSocket->bind( QHostAddress::AnyIPv4,
m_p_striderSettings->destUDPPort,
QUdpSocket::ShareAddress | QUdpSocket::ReuseAddressHint

Do you need Shareaddress and ResuseAddressHint? You have some whatever port you
chooose and you do not need this. I do not see in documentation this as third parameter? I mean QUdpSocket::ShareAddress | QUdpSocket::ReuseAddressHint...

You said:

"
while( m_p_UDPSocket->hasPendingDatagrams() ) {
// Process the data
}

"Is the while loop blocking the QUDPSocket from getting more data
packets since it is in the same thread? Does anyone
know of a good approach to a high performance QUDPSocket receiver?"
"

Yes!


if (mainudp->hasPendingDatagrams())
{
quint64 size = mainudp->pendingDatagramSize();
buffer.resize(size);
mainudp->readDatagram(buffer.data(), size, &sender, &senderPort);
//qDebug() << sender << senderPort;
}

of course you need to have:

QMetaObject::Connection ret = connect(mainudp, SIGNAL(readyRead()), this, SLOT(readyReading()));
What do you think?

One packet by one and signal which SAYS YOU HAVE SOMETHING TO READ! That might be solution.. I guess!

[Pereubu2018]

You wrote "Adding an event loop makes it work! I guess the event loop allows the socket to read from the OS buffer without blocking when in the while loop?"

while loop will take a lot of resources. It will block everything! Even when it is waiting! But have a look on documentation and its example:

void Server::initSocket()
{
udpSocket = new QUdpSocket(this);
udpSocket->bind(QHostAddress::LocalHost, 7755);

connect(udpSocket, SIGNAL(readyRead()),
this, SLOT(readPendingDatagrams()));
}

SO what is the impact of this (taken from documentation):

void QIODevice::readyRead()
This signal is emitted once every time new data is available for reading from the device's current read channel. It will only be emitted again once new data is available, such as when a new payload of network data has arrived on your network socket, or when a new block of data has been appended to your device.
readyRead() is not emitted recursively; if you reenter the event loop or call waitForReadyRead() inside a slot connected to the readyRead() signal, the signal will not be reemitted (although waitForReadyRead() may still return true). (MY COMMENT: if that is so, why documentation gives while loop)
Note for developers implementing classes derived from QIODevice: you should always emit readyRead() when new data has arrived (do not emit it only because there's data still to be read in your buffers). Do not emit readyRead() in other conditions.

IT HARD FOR ME TO UNDESTAND. Let say I sending very fast data (I might use maybe
void Server::readPendingDatagrams()
{
while (udpSocket->hasPendingDatagrams()) {
QNetworkDatagram datagram = udpSocket->receiveDatagram();
processTheDatagram(datagram);
}
}

So the signal from udp socket will be emitted when? What about this option:
connect(udpSocket, SIGNAL(readyRead()), this, SLOT(readyRead()), Qt::QueuedConnection)??? Let say if (not while) will process every packet coming into sockets. What if packets are two fast and one has come while the previous are still in buffer? What would be with and what without QueuedConnection? What I am going to read with slot reading one fragment of receiving data? Because with if I am expecting one fragment of data?

It is still confusing for me!

---

Added after 8 minutes:

I do not see how you implemented multithreading and if that is need here? I want to know when emit signal for connect really hits. I know when it is emitted but when it is transfereed to slot?