hpx icon indicating copy to clipboard operation
hpx copied to clipboard

Remove need for HPX_PLAIN_ACTION macro.

Open FunMiles opened this issue 4 years ago • 6 comments

As mentioned in https://github.com/STEllAR-GROUP/hpx/issues/3992 there is a lot of boilerplate in HPX. And macros are generally evil, so removing them step by step is in my opinion a good idea.

I looked at UPC++ and they offer a much more flexible way for RPC calls. They allow certain lambdas (with only trivially copyable captures). Though there is no standard guarantee that the lambda capture itself is trivially copyable, at least for empty captures, there should be no difficulty in making the call to a remote lambda. The question is how to automatically register a function or a lambda from the existence of the call itself. I believe it is possible since C++14 thanks to the template variables. Here is an rough outline of the approach I think could do the trick:

template <typename X, typename Signature>
auto registerLambda() {
    using ArgTuple = TupleFromArg_t<Signature>;
    using R = ReturnFromSignature_t<Signature>;
    auto apply = std::function<R(ArgTuple &)>{ 
          /// tuple expanding call to the an empty lambda
   };
/// More things needed... to register the function(s) with the typeid(X).name(), but nothing conceptually difficult.
    return std::tuple{
        std::string{typeid(X).name()},
        apply
    };
}

template <typename X, typename Signature>
auto registration_info = registerLambda<X,Signature>();

/// RPC for an empty-capture lambda example
template <typename Ftor, typename ...Arg>
auto rpc(Ftor &&ftor, Arg&& ... arg) {
	using R = decltype(ftor(std::decay_t<Arg>(arg)...));
	const auto &info = my_id_info<std::decay_t<Ftor>, std::decay_t<R>(std::decay_t<Arg>...)>;
        return detail::rpc(info, std::forward<Arg>(arg)...);
}

Variation of that can be done. As for functions, creating a template with the function pointer would easily do the trick as we.. Unless someone knows better (I am not up on all the legalese of C++14 and later), the registration is done at loading of the code and it is not necessary for any process to actually make a call to the rpc for it to happen.

FunMiles avatar Aug 04 '21 18:08 FunMiles

Please see: https://github.com/STEllAR-GROUP/hpx/blob/master/libs/full/actions_base/include/hpx/actions_base/lambda_to_action.hpp. Is that what you're after? Also here is an example on how to use it: https://github.com/STEllAR-GROUP/hpx/blob/master/libs/full/async_distributed/tests/regressions/plain_action_1550.cpp.

hkaiser avatar Aug 04 '21 18:08 hkaiser

Please see: https://github.com/STEllAR-GROUP/hpx/blob/master/libs/full/actions_base/include/hpx/actions_base/lambda_to_action.hpp. Is that what you're after? Also here is an example on how to use it: https://github.com/STEllAR-GROUP/hpx/blob/master/libs/full/async_distributed/tests/regressions/plain_action_1550.cpp.

I am going to look into this deeper the two link you gave.

However, I don't think it goes as far as what I want. It still requires to wrap the lambda with a call to hpx::actions::lambda_to_action. I am looking at being able to make a rpc call with a lambda that does not need to have been manually registered. In the outline I gave above, the simple fact that the code contains call to rpc([](auto a, auto b){ return a+b;}, 1, 2); triggers the instantiation of the template variable registration_info<decltype([](auto a, auto b){ return a+b;})> and thus a call to registerLambda with the appropriate lambda type. This means that one can write what the code is meant to do closer to where it is being used, rather than somewhere above like in the example of using lambda_to_action.

Closer to HPX's syntax, I'd like to call hpx::async(remote_node, [](auto a, auto b){ return a+b;}, 1, 2); rather than: something like

/// Somewhere above
static action = hpx::lambda_to_action( [](auto a, auto b) {return a+b;});

/// Now in my routine
    hpx::async(action, remote_node, 1,2);

The lamdba_to_action is still too much boiler plate in my view of writing clean easy to write and follow code.

PS: I am a novice to HPX. I had seen your previous presentation. I have been advocating task-driven parallel code since 1993 or so. Thus I certainly like the concepts of HPX. I am suddenly driven to HPX more eagerly by your presentation at CPPcon 19 with the co_await. It is that kind of clear and clean code version I am after. Boiler-plate registration and such are a distraction.

FunMiles avatar Aug 04 '21 21:08 FunMiles

PS: On the other hand, I have the impression that I can achieve what I want with a very simple use of hpx::lambda_to_action at least for lambdas with an empty capture. It seems that if I use the template variable to call hpx::lambda_to_action, then I will have reached my goal. There's only one mystery left. Why is there a call to std::move ? If it is needed then the action can only be used once? Or is it not really moved but it is there to make sure the correct overload of hpx::async is being used?

FunMiles avatar Aug 05 '21 15:08 FunMiles

There's only one mystery left. Why is there a call to std::move ?

All actions can be invoked more than once, but each invocation will receive the arguments via std::move.

I'll try to answer all the other questions and comments later.

hkaiser avatar Aug 05 '21 16:08 hkaiser

I have created a quick test and the following essentially does what I want:

template <typename Lambda>
auto self_action = hpx::actions::lambda_to_action( Lambda{} );

template <typename Lambda>
auto wrapped(Lambda &&l) {
	return self_action<Lambda>;
}

int main()
{
    // Get a list of all available localities.
    std::vector<hpx::naming::id_type> localities = hpx::find_all_localities();

    // Reserve storage space for futures, one for each locality.
    std::vector<hpx::lcos::future<void>> futures;
    futures.reserve(localities.size());

     for (hpx::naming::id_type const& node : localities)
    {
        // Asynchronously start a new task. The task is encapsulated in a
        // future, which we can query to determine if the task has
        // completed.
        futures.push_back(hpx::async(wrapped([]{std::cout << "Lambda running on " << std::this_thread::get_id() << std::endl;}), node));
    }

    // The non-callback version of hpx::lcos::wait_all takes a single parameter,
    // a vector of futures to wait on. hpx::wait_all only returns when
    // all of the futures have finished.
    hpx::wait_all(futures);
    return 0;
}

I will wait for more info about the std::move, but given the way my function wrapped returns the variable value, I presume there is no need for std::move. Note that it is my understanding that what I wrote above only works in C++20 because before then, the default constructor of the closure is either deleted (C++11) or absent (C++14 C++17). I do think one can hack around that, though I am not sure if it would really trigger some undefined behavior. At least for C++20 an empty capture does have a default constructor and I believe that what I wrote is standard compliant. Before then, one could argue that for system where the closure satisfies trivially copyable, one can make use of the copy constructor with a reinterpret_cast<const Lambda &>(closureSpace). It is even possible, in this case, to transfer the closure to the remote system to cover the non-empty closure case. I believe this is what UPC++ must be doing.

Edit: I realize that the <C++20 and capture idea are hacks. They may work with most compilers but are not guaranteed by the standard. However for C++20, given that the standard guarantees the default constructor, I'd be happy to use that and just pass what could have been in the closure as arguments.

Edit 2: Capture-less lambda have always been convertible to function pointers, so this would be a hack-free approach even with C++17 but at the price of declaring the lambda as a constexpr variable.

FunMiles avatar Aug 06 '21 19:08 FunMiles

@NK-Nikunj you mentioned at some point that you have this implemented locally. Could you share your implementation, please?

hkaiser avatar Mar 11 '22 22:03 hkaiser