Signal/Slot design philosophy

J.Hilk · 17 Oct 2018, 07:39

Hi everyone,
I'll like to pitch a scenario and would like to know your opinion and the reason behind it.

Scenario:

Imagine a a (QObject based) class - lets name this StateClass - that monitors states and emit signals when states change and conditions are met. That class takes a QVector<int> in it's update function.

The parent class - StateManager reacts to the signals and shows reactions in the GUI, it also has multiple - let's say 4 - instances of the above class to monitor different stuff.

My question:
How do you pass the data from a 3rd class (a sibling to the StateManager, has to be for reasons) that gets new data from some kind of communication port to the StateClass instances.

Do you
a) define a signal something(QVector<int>) in StateManager and define QObject::connect, to connect that Signal to the StateClasses

b) define a function - in StateManager - that passes the new data to the StateClass instances one after the other.

c) do something totaly different.

I'm undecided.
It's even hard to decide what would be faster (of a or b), as long as you use Qt5 connects. Who knows what the compiler rationalises away.

sierdzio · wrote on 17 Oct 2018, 06:12

I'd vote for a).

But, with some modification - because (as I understand) that sibling class may not know the full state of the StateClass. So perhaps it won't be able to send the full QVector<int> - I think a separate update signal/slot is necessary, one which would specifically say which state was changed.

kshegunov · replied to J.Hilk on 17 Oct 2018, 07:39

@J.Hilk said in Signal/Slot design philosophy:

a) define a signal something(QVector<int>) in StateManager and define QObject::connect, to connect that Signal to the StateClasses

Most definitely. There are few reasons:

Decoupling: Suddenly I want that StateManager to start notifying some other class too, with b) that ain't happening as easily. With a) I'm just making a couple of more connects.

1.a) Threading: From 1) follows that if I want to put some of those objects into a thread I'm good to go from the start. Otherwise I'm in for a bad surprise.

1.b) Ownership: As it often happens some or all of those StateClass objects may not be owned by the StateManager instance. Then it becomes a real pain in the ass to keep track of them, when they get constructed, to register them, when they get deleted and so on. Qt already does that for you with the signal-slot connections, as they vanish when the object dies.
Encapsulation: More often than not such objects may be stand-alone (see also 1.b). Then I'm exposing a whole lot of an interface to StateManager to bookkeep the lot of them, and in it I'm making my life miserable by holding references to objects that I don't own. I would ideally want each object of each class to be completely self-sustaining.
Debugging: This is a drawback. When you have gazillion of connections going all 'round the place it makes debugging harder. And some weird behavior that you encounter may not be immediately visible from the code. Especially true when you queue them through the event loop. This is actually my current problem with the QDateTimeEdit - I have wired all kinds of weird stuff around and have a strange bug with my custom control; yet to be determined why ...

kshegunov · 17 Oct 2018, 07:58

PS.

It's even hard to decide what would be faster (of a or b), as long as you use Qt5 connects. Who knows what the compiler rationalises away.

This is an afterthought usually, but say we take a stab at it for the fun.

With direct connections you're wasting almost nothing. You get a list of pointer-to-members and start executing stuff one by one (that's pretty much what Qt does). So that performance hit is negligible, think what's the hit of "std::bind" ... a function call? No one would optimize that.
With queued connections you have a bit more of a performance hit, but ordinarily you wouldn't queue stuff when working in single thread. The multithreaded case is a bit more interesting, but you can sacrifice the event loop "inefficiency" (i.e. all events' access being serialized through a single queue) for the cleanliness and convenience. You can squeeze a bit more if you're willing to write imperative with QThread::run and sync primitives, but I'd reserve that for sensitive code that is not event driven to begin with (like crunching some numbers).

sierdzio · 17 Oct 2018, 08:23

@kshegunov said in Signal/Slot design philosophy:

With queued connections you have a bit more of a performance hit, but ordinarily you wouldn't queue stuff when working in single thread. The multithreaded case is a bit more interesting, but you can sacrifice the event loop "inefficiency" (i.e. all events' access being serialized through a single queue) for the cleanliness and convenience. You can squeeze a bit more if you're willing to write imperative with QThread::run and sync primitives, but I'd reserve that for sensitive code that is not event driven to begin with (like crunching some numbers).

I also recommend the queued connection in case of threads. It avoids all mutex hassle and forces nice object separation (only signal-slot connections, no direct calls to the thread).

kshegunov · replied to sierdzio on 17 Oct 2018, 08:23

@sierdzio said in Signal/Slot design philosophy:

I also recommend the queued connection in case of threads. It avoids all mutex hassle and forces nice object separation (only signal-slot connections, no direct calls to the thread).

I was talking more like using Qt::AutoConnection than anything here. However I'd want to open a bracket and claim that it's not unreasonable to use both in a threaded environment. I mean threading is a big universe and how you approach a problem is ... well ... a complex topic. But consider the following snippet (excerpt from a current project):

class RbSqlJob : public QObject
{
    Q_OBJECT
    Q_DISABLE_COPY(RbSqlJob)

public:
    // ...
    void start();               //!< \threadsafe
    void cancel();              //!< \threadsafe
    bool isCanceled() const;    //!< \threadsafe

    void dataReady(const RbSqlData &);

protected:
    virtual void run(QSqlDatabase &) = 0;  //< This is run in a separate thread as a method called by a slot (think worker object)

    // ...

private:
    enum { Running = 1, Canceled };
    QAtomicInt status;
};

The crux of the issue here is that I want to be able to "abort" a processing function if it's running in a thread. However I wouldn't want to split all the code around the place because it'd become pretty unmanageable. And run can take some time to process the data. So the 3 functions go about roughly like:

void RbSqlJob::start()
{
    if (status.load() == Running)
        return;

    status.store(Running);
    // ... more code ...
}

void RbSqlJob::cancel()
{
    if (status.load() != Running)
        return;

    status.store(Canceled);
    // ... more code ...
}

bool RbSqlJob::isCanceled() const
{
    return status.load() == Canceled;
}

Then usage is like:

    SomeJob * job = new SomeJob(...);

    // Notice that forcing DirectConnection is imperative here (as the event loop is likely blocked in the worker thread).
    QObject::connect(dialog, &QProgressDialog::canceled, job, &RbSqlJob::cancel, Qt::DirectConnection);  
    QObject::connect(job, &RbSqlJob::finished, dialog, &QProgressDialog::deleteLater);
    // ... more connects to utilize the data transfer and such ...

    job->start(); //< Does some magic to move the object to the correct thread

And while the worker object is still moved to the SQL thread, as is usual, there's also the odd direct connection to manage the code in a responsive fashion when the worker thread's event loop is blocked ...

Well that post became a novel, so I'm going to stop here.

J.Hilk · 20 Oct 2018, 08:30

Hi all,

first of, thanks @kshegunov and @sierdzio for your input.
You both gave great new points of view I hadn't really considered yet.

Most of it won't effect the real life example I have right now, but still valid in a general sence.

I especialy appreciate the threaded example that makes use of an explicet Qt::DirectConnection

I personally also would go with option a), And thats why I will change the code right away.
To bad CompilerExplorer doesn't work with qt libaries. Would be nice to see the assembler code of a Signal&Signal connection and slot&function call.

I'll go ahead and close the question, thanks again for the input!

kshegunov · 22 Oct 2018, 04:41

@J.Hilk said in Signal/Slot design philosophy:

To bad CompilerExplorer doesn't work with qt libaries. Would be nice to see the assembler code of a Signal&Signal connection and slot&function call.

You can do that from creator. :)

J.Hilk · 22 Oct 2018, 07:24

@kshegunov said in Signal/Slot design philosophy:

@J.Hilk said in Signal/Slot design philosophy:

To bad CompilerExplorer doesn't work with qt libaries. Would be nice to see the assembler code of a Signal&Signal connection and slot&function call.

You can do that from creator. :)

WHAT!? Do tell, I'm unaware of that feature.

kshegunov · replied to J.Hilk on 22 Oct 2018, 07:24

@J.Hilk said in Signal/Slot design philosophy:

WHAT!? Do tell, I'm unaware of that feature.

Debug > Operate by Instruction

Unless you had something else in mind.