Overhead when transmitting same signal to multiple threads via QueuedConnection

[ghorwin]

Hi all,

I've connected a main thread's signal to the same slot of many objects (of same type) that all live in their own thread. I'm using QueuedConnection. The signal transports a potentially large argument (QJsonObject).



signals:
/*! Signal signature. */
void sendData(QJsonObject o);


Actually, only one thread is the receiving thread and all the other threads simply ignore the message.

From my understanding of Qt, the payload of the signal is copied over into the target thread's memory where it can be accessed from the target thread's slot without any locking mechanism.
If I now copy the payload needlessly to many threads (all but one discard the message), I create an unnecessary overhead.

Questions:
1. is my expectation on Qt's behavior regarding copying of payload during queuing of the signal correct?
2. is there an alternative design which allows me to emit the signal such, that it is only send to one of the connected threads? (i.e. connecting the target thread before emit and disconnecting afterwards would work, but has an overhead as well, right?)

Thanks for any insights,
Andreas

[Lesiok]

1. Who performs connect: sender or receiver ?
2. Who decides whether to ignore data: sender or receiver ?

[d_stranz]

If I now copy the payload needlessly to many threads (all but one discard the message), I create an unnecessary overhead.

You are passing your QJsonObject instance by value: sendData( QJsonObject o ). In C++, this requires making a copy of the data. If you pass by reference instead: sendData( QJsonObject & o ) or sendData( const QJsonObject & o ) if the object will not be modified during the call, then you avoid copying.

And as Lesiok asks, if you are in control of the connections, then why are you making connections to receivers who ignore the signal?

[anda_skoa]

if the object will not be modified during the call, then you avoid copying.


In general this is true, but not in the case of a QueuedConnection.

That mechanism requires that the signal arguments are stored in QVariant as the event being used for the delayed/cross-thread invocation uses that to store the arguments of the call.

However, if we look at QJsonOject's copy constructor (https://code.woboq.org/qt5/qtbase/src/corelib/serialization/qjsonobject.cpp.html#_ZN11QJsonObjectC1ERKS_) we see that it uses reference counting instead of copying the data itself.

Which of course also means that none of the threads, including the main thread, should attempt any modification of the object as it is now shared data.

Cheers,
_

[ghorwin]

Hi,

thanks for the answers, let me clarify:

@Lesiok:
- sender performs connect, threads already exist and are started, QueuedConnection is created
- all but one receivers (threads) rejects/ignores signal unless target matches

@d_stranz:
- copy by value was intended, otherwise we deal with shared data (as pointed out by anda_skoa) and I have to deal with blocking access to the shared object until receiver clears object. I wish to avoid potentially dangerous locking constructs by design.
- the receivers are connected right after the objects are created (done dynamically at runtime based on runtime conditions)

@anda_skoa:
- thanks for looking into the constructor code - for some reason I was believing that Qt's object constructors perform a deep copy when passing data between threads. So, my current design does not have a performance overhead, but is risking race conditions/undefined behavior due to undetermined access to the shallow-copied JSonObject.


My new solution:

Create a deep copy once before emitting the signal


// deep copy of QJsonObject stored in a document that resides in main thread's memory
QJsonObject o = QJsonObject::fromVariantMap(document().object().to VariantMap());
// safely send to other thread
emit jsonAvailable(o);
// when o goes out of scope, sending thread cannot access/disturb it anylonger, but the object remains and only receiving thread will access it.


Now the signal can be broadcasted to all listening threads and only the intended audience will actually look at the object.

Any suggestions on how to improve that?

Cheers,
Andreas

[d_stranz]

Thanks for the clarification.


Any suggestions on how to improve that?

Actually, this sounds pretty good. Making the deep copy protects the original, the copy will be passed with the least overhead, and if only one receiver will actually handle the signal, the copy cannot be corrupted.

[tuli]

Which of course also means that none of the threads, including the main thread, should attempt any modification of the object as it is now shared data.

Does that mean a copy of reference-counted object is not actually threadsafe?

If I have a QVector, and I passs a simple copy to another thread, that's a problem? I have to make sure that an actual deep copy is made?

[d_stranz]

If I have a QVector, and I pass a simple copy to another thread, that's a problem? I have to make sure that an actual deep copy is made?

QVectors have copy-on-write semantics: From the docs:


QVector::QVector(const QVector<T> & other)
Constructs a copy of other.
This operation takes constant time, because QVector is implicitly shared. This makes returning a QVector from a function very fast. If a shared instance is modified, it will be copied (copy-on-write), and that takes linear time.

and

T & QVector:: operator[](int i)
Returns the item at index position i as a modifiable reference.
i must be a valid index position in the vector (i.e., 0 <= i < size()).
Note that using non-const operators can cause QVector to do a deep copy.


So if you pass a QVector by value to another thread, and that thread modifies it, then before the modification takes place a deep copy will be made. That means that the copy held by the thread will no longer the same as the original.

There is a good article by Marc Mutz (https://marcmutz.wordpress.com/effective-qt/containers/) comparing STL and Qt containers, with an example about a third of the way down on how to modify containers in a thread-safe way.

[ghorwin]

Hi,

so, we have now a "best practice" approach when dealing with shallow-copy Qt objects being passed between threads.

First, create a deep copy of the object, connect the signal to the one receiving object, emit the signal and disconnect again. This way we handle ownership transfer and ensure that only one object is copied/accessed.



// create deep copy of object as local object
QJsonObject clonedObject = QJsonObject::fromVariantMap(o.toVariantMap());
// create a queued connection to socket's thread
connect( this, SIGNAL(sendJsonMessage(QJsonObject)),
socket, SLOT(sendJsonObject(QJsonObject)),
Qt::QueuedConnection);
// transfer the data
emit sendJsonMessage(clonedObject);
// disconnect the signal
disconnect( this, SIGNAL(sendJsonMessage(QJsonObject)),
socket, SLOT(sendJsonObject(QJsonObject)));

// at end of function, local object goes out of scope and ownership is transfered to receiving thread


-Andreas

[anda_skoa]

Why the connect and disconnect?

Cheers,
_