palacaze/sigslot

Invocation order

felaugmar opened this issue · 7 comments

I would like to give a certain invocation order to the connected slots, I saw the Boost.Signals2 solves the need with the Ordering Slot Call Groups, which seems fine, but I don't know about implementation details of it against the sigslot, it's possible to implement a similar feature?

In Signals2 you can connect in different orders like:

sig.connect(1, World());  // connect with group 1
sig.connect(0, Hello());  // connect with group 0

Which calls Hello() and then World(), but for my needs an incremental order (without giving the group number) would be enough.

I do not know if it would help with your problem, but a minimalist solution would be the following:

#include <sigslot/signal.hpp>
#include <functional>
#include <iostream>
#include <vector>

template <typename... Args>
struct slot_group : public std:: vector<std::function<void(Args...)>> {
    template <typename... U>
    void operator()(U && ...u) const {
        for (const auto &f : *this) {
            f(std::forward<U>(u)...);
        }
    }
};

int main() {
    slot_group<std::string, int> group;
    group.push_back([] (std::string s, int i) {
        std::cout << "First to print " << s << " and " << i << std::endl;
    });
    group.push_back([] (std::string s, int i) {
        std::cout << "Second to print " << s << " and " << i << std::endl;
    });

    sigslot::signal<std::string, int> sig;
    sig.connect(group);
    sig("bar", 1);
    return 0;
}

The solution doesn't help much, it works, but I miss sigslot features, like connection management, #6 and automatic slot lifetime tracking for each slot in the group.

Implementing this feature would not be very hard, but I worry it would have a steep impact on emission performance.

I agree it may be a very important feature in some circumstances, I will make an attempt in a feature branch and we will benchmark it.

I pushed a very simple implementation for that feature on the slot_group branch.

It is api-compatible so existing code does not need changing. Preliminary benchmarks do not seem to report outstanding performance regressions.

I may need this feature in complex scenarii, so I definitively want to merge it once I am confident it won't negatively impact performance. Then again, I favor correctness and features over execution speed.

You did to store the groups in a std::vector, the impact of storing it in a std::set would be too high?

I mean, connecting and disconnecting will take more time, but the emission performance would be better than in a std::vector with empty groups.

Yes, the cost std::set would be very high. At the very least if I wanted arbitrary slot group ids I would use a sorted vector.

In real-life code (as opposed to micro-benchmarks), I agree that connection and disconnection happen rarely compared to emission, and code should be optimized towards this.

I will test a few approaches, I think keeping the vector of slots sorted by slot group ids may be the best option, it allows arbitrary id values and only needs one container (instead of the vector of vectors of my proof of concept).

I push one last update to allow arbitrary integer ids. This is on master now.

If all goes well I will tag 1.2 in a few days.