Serialization of global_fnptr<Fn,function_token> overestimates needed size

[Colin MacLean]

The custom serialization of global_fnptr<Fn,function_token> doesn’t include ubound and therefore command uses the size of function_token, which has room for the 20 byte hash. This could make decisions about message type less efficient as the maximum message size could be unnecessarily pushed over message size limits into slower AM types.

[Colin MacLean]

This issue is more difficult to solve than it initially appeared. command::ubound needs to be changed to use cat_ubound_of(std::declval<executor_wire_t>()) and we don’t have the executor_wire_t (global_fnptr) instance, yet. That’s created inside of command::serialize().

There’s a bit of a chicken and the egg problem:

Sketch:

template<typename Fn, /* ... */, typename... Args>
struct command_serializer // Has to be a new type because we need to know additional types which are erased in command
{
  command_serializer(const typename std::remove_reference<Fn>::type& f) // lvalue, but type of Fn could cause a move to happen later.
    : fn(std::addressof(f))
  {}
  void serialize()
  {
    static_cast<Fn&&>(*fn); //uses correct reference type. Will cause a delayed move if Fn isn't an lvalue reference
  }
  typename std::remove_reference<Fn>::type* fn;
};

For instance,

    auto ub = detail::command<detail::lpc_base*>::ubound(
      detail::empty_storage_size.cat_size_of<bcast_payload_header>(),
      fn
    );
    detail::command<detail::lpc_base*>::template serialize<
        bcast_as_lpc::reader_of,
        bcast_as_lpc::template cleanup</*definitely_not_rdzv=*/definitely_not_rdzv>
      >(w, ub.size, fn);

at first glance looks like it could be rewritten

    detail::command_serializer<Fn1&&, bcast_as_lpc::reader_of, bcast_as_lpc::template cleanup</*definitely_not_rdzv=*/definitely_not_rdzv, detail::lpc_base*> cmd(fn);
    auto ub = cmd.ubound(detail::empty_storage_size.cat_size_of<bcast_payload_header>());
    cmd.serialize(w, ub.size); // Will move `fn` if Fn1&& is an rvalue

except that definitely_not_rdzv is based on ub, which isn’t known until after the function token is created. I’m not seeing a solution to this problem. We can’t decide on the correct deserialization function until the ubound is known and we can’t calculate the ubound until we have the token for that deserialization function. This overestimation doesn’t seem possible to fix.

[Dan Bonachea]

Deferred until we find an acceptable solution approach