Add serialization through `std::reference_wrapper`

Issue #199 resolved
Dan Bonachea created an issue

Currently we serialize "through" references, ie: (spec 6.4-{1,2})

  • A reference type cq T& or cq T&& is Serializable when T is Serializable. Such a type is serialized by serializing the referent, and deserialization produces an object of non-reference type deserialized_type_t<T>. Thus, deserialized_type_t<cq T&> and deserialized_type_t<cq T&&> are both the same as deserialized_type_t<T>.
  • A reference type is never TriviallySerializable.

However we have not extended this to include the std::reference_wrapper<T> utility class that is often used to contain references. Worse still, std::reference_wrapper<T> is usually TriviallyCopyable (required starting in C++17), which means we consider it TriviallySerializable (which it semantically is not, because the implementation contains a raw pointer). As a result, sending a std::reference_wrapper<T> via RPC to any other process currently silently results in garbage.

Example program:

#include <upcxx/upcxx.hpp>
#include <iostream>
#include <string>
#include <functional>

int main(int argc, char *argv[]) {
    upcxx::init();

    std::string s = "Hello from rank ";
    s += std::to_string(upcxx::rank_me());

    if (!upcxx::rank_me()) std::cout << upcxx::is_trivially_serializable<decltype(s)>::value <<std::endl;
    upcxx::barrier();
    upcxx::rpc(0,[](std::string const &s1) {
      std::cout << s1 << std::endl;
    }, s).wait();
    upcxx::barrier();

    std::string &s_ref = s;
    if (!upcxx::rank_me()) std::cout << upcxx::is_trivially_serializable<decltype(s_ref)>::value <<std::endl;
    upcxx::barrier();
    upcxx::rpc(0,[](std::string const &s1, std::string const &s2) {
      UPCXX_ASSERT_ALWAYS(s1 == s2);
      std::cout << s2 << std::endl;
    }, s, s_ref).wait();
    upcxx::barrier();

    std::reference_wrapper<std::string> s_refwrap = s;
    if (!upcxx::rank_me()) std::cout << upcxx::is_trivially_serializable<decltype(s_refwrap)>::value <<std::endl;
    upcxx::barrier();
    upcxx::rpc(0,[](std::string const &s1, std::string const &s2) {
      UPCXX_ASSERT_ALWAYS(s1 == s2);
      std::cout << s2 << std::endl;
    }, s, s_refwrap).wait();
    upcxx::barrier();

    upcxx::barrier();
    if (!upcxx::rank_me()) std::cout << "SUCCESS" <<std::endl;
    upcxx::finalize();
    return 0;
}

Example Output from 2022.3.0:

upcxx -network=smp -g ref-serialize.cpp && upcxx-run -n 4 a.out
0
Hello from rank 0
Hello from rank 2
Hello from rank 1
Hello from rank 3
0
Hello from rank 0
Hello from rank 3
Hello from rank 1
Hello from rank 2
1
Hello from rank 0
*** FATAL ERROR (proc 0): 
//////////////////////////////////////////////////////////////////////
UPC++ assertion failure:
 on process 0 (pcp-d-10)
 at ref-serialize.cpp:32
 in function: main(int, char**)::<lambda(const string&, const string&)>()

Failed condition: s1 == s2

To have UPC++ freeze during these errors so you can attach a debugger,
rerun the program with GASNET_FREEZE_ON_ERROR=1 in the environment.
//////////////////////////////////////////////////////////////////////

This seems like a simple oversight on our part. If we serialize through references T& and T&& then we should apply the same policy to std::reference_wrapper<T> and deploy appropriate serialization logic to make that happen.

The proposed change would presumably imply asymmetric deserialization for std::reference_wrapper<T>, in the same way that we have asymmetric deserialization now for T& and T&&. IOW we'd need to explain that:

deserialized_type_t<std::reference_wrapper<T>> == deserialized_type_t<T>

which (due to our normal serialization rules) has implications for the containers where this utility is frequently used - for example:

deserialized_type_t<std::vector<std::reference_wrapper<T>>> == std::vector<deserialized_type_t<T>>

ie, the asymmetry propagates outward to containers (something that might need to be explained in the guide).

Comments (5)

  1. Dan Bonachea reporter

    As mentioned in the Impl PR:

    The spec for this needs to make clear that we strip const qualifiers from inside std::reference_wrapper (and regular references). The current spec wording for this detail is IMO a bit too subtle and should be clarified. Currently 6.4-1 and 6.4-4 could be misread as mutually contradictory. This is probably just a matter of being a bit more explicit, and possibly reordering the 6.4 paragraphs for clarity.

  2. Log in to comment