tdlib / td

Cross-platform library for building Telegram clients
https://core.telegram.org/tdlib
Boost Software License 1.0
7.11k stars 1.44k forks source link

Merge ImmediateClosure and DelayedClosure to Closure #2363

Open yanminhui opened 1 year ago

yanminhui commented 1 year ago

https://github.com/tdlib/td/blob/70bee089d492437ce931aa78446d89af3da182fc/tdutils/td/utils/Closure.h#L56-L118

It seems ImmediateClosure and DelayedClosure can merge to one class, for example (see Insights):

#include <tuple>
#include <type_traits>

template <class T>
struct MemberPointerClass {};

template<class T, class U>
struct MemberPointerClass<T U::*> {
    using type = U;
};

template <class F, class... Args>
class Closure {
    template <class FT, class... ArgsT>
    friend auto MakeImmediateClosure(FT f, ArgsT&&... args);

    template <class FT, class... ArgsT>
    friend auto MakeDelayedClosure(FT f, ArgsT... args);

    explicit Closure(F f, Args... args)
    : f_{f}, args_{std::forward<Args>(args)...}
    {}

public:
    template <class Object>
    [[maybe_unused]] auto run([[maybe_unused]] Object* obj) const {
        if constexpr (std::is_member_function_pointer_v<F>) {
            using C = std::conditional_t<std::is_const<Object>::value,
                                        std::add_const_t<typename MemberPointerClass<F>::type>,
                                        typename MemberPointerClass<F>::type>;
            static_assert(std::is_base_of_v<Object, C>);
            return std::apply(f_, std::tuple_cat(std::make_tuple(static_cast<C*>(obj)), std::move(args_)));
        } else {
            return run();
        }
    }

    [[maybe_unused]] auto run() const {
        return std::apply(f_, args_);
    }

private:
    F f_;
    std::tuple<Args...> args_;
};

template <class F, class... Args>
auto MakeImmediateClosure(F f, Args&&... args) {
    return Closure<F, Args...>(f, std::forward<Args>(args)...);
}

template <class F, class... Args>
auto MakeDelayedClosure(F f, Args... args) {
    return Closure<F, std::decay_t<Args>...>(f, std::move(args)...);
}
levlam commented 1 year ago

Likely, yes, but TDLib is written in C++14, hence we can't use std::apply and other minor features used in the PoC code.

Also, what benefits do you try to achieve by merging the classes?

yanminhui commented 1 year ago

https://github.com/tdlib/td/blob/70bee089d492437ce931aa78446d89af3da182fc/tdactor/td/actor/impl/Event.h#L56-L98

LambdaEvent can be replaced with ClosureEvent as well. Using lambda and member function pointer the same way.

int a = 1;
int b = 2;

const Base* obj = new Derive{};
auto c = MakeImmediateClosure(&Derive::add, a, b);
std::cout << "run memptr: " << c.run(obj) << std::endl;
delete obj;

auto c2 = MakeImmediateClosure([](int a, int b) { return a + b + 4; }, a, b);
std::cout << "run lambda: " << c2.run(obj) << std::endl;  // or c2.run(), obj is ignored.
levlam commented 1 year ago

It is definitely possible to achieve the same behavior in many ways. But is there reason to change the current implementation?

yanminhui commented 1 year ago

It is for simplify logic code, and usage consistency for lambda and member pointer function only.

In addition, the events that send to actor is mostly custom events (i.e. member pointer functor), it can improve performance like std::function by allocate object on the stack.

_FunAlloc __af(__a);
if (__use_small_storage<_Fun>())
{
    ::new ((void*)&__buf_.__small)
        _Fun(_VSTD::move(__f), _Alloc(__af));
}
else
{
    typedef __allocator_destructor<_FunAlloc> _Dp;
    unique_ptr<_Fun, _Dp> __hold(__af.allocate(1), _Dp(__af, 1));
    ::new ((void*)__hold.get())
        _Fun(_VSTD::move(__f), _Alloc(__af));
    __buf_.__large = __hold.release();
}