palacaze / sigslot

A simple C++14 signal-slots implementation
MIT License
709 stars 97 forks source link

Invocation order #14

Closed ghost closed 4 years ago

ghost commented 4 years ago

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.

palacaze commented 4 years ago

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;
}
ghost commented 4 years ago

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.

palacaze commented 4 years ago

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.

palacaze commented 4 years ago

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.

ghost commented 4 years ago

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.

palacaze commented 4 years ago

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).

palacaze commented 4 years ago

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.