Passing Multiple IDeserializationContext objects to Group Constructor.

[songweijia]

When we construct a subgroup object(Replicated) from transferred states, T::from_bytes() sometime needs an IDeserializationContext object regarding type T. I tried mutils::KindMap<> to contain multiple type-related contexts in a single IDeserializationContext object so that T::from_bytes to retrieve the corresponding one on demand. But that approach requires that T::from_bytes is aware of all Types in mutils::KindMap<>, which is impossible at the moment the group is constructed (and there could be various combinations of all the types.). Therefore I decided to all the Group constructor to accept a vector of IDeserializationContext objects by pointers. The code is implemented in commit 8ecd36ff74fecc43a51c7bbfa03c76a42cd88d97.

I plan to merge it to master, please let me know what do you think.

[etremel]

If I understand correctly, you're saying that a user of Derecho might want to supply a different IDeserializationContext for each subgroup type, whereas our current Group constructor only allows the user to provide a single IDeserializationContext that will be used for all subgroup objects. This makes sense to me, since as far as I remember the original intention of the Deserialization Context (from mutils) was for it to represent some kind of "helper object" needed to deserialize an object; obviously it might be necessary to have a different Deserialization Context for each class of object that you deserialize.

However, there are a few things about this proposal that confuse me:

1. What is the point of the IDeserializationContext type in the first place? It looks like it is simply a new name for the type mutils::RemoteDeserializationContext in the derecho namespace, but it introduces an extra level of inheritance and adds the useless letter "I" to the typename. (I generally dislike Hungarian notation because it pollutes the name of a type with language-syntax information). If we wanted to put RemoteDeserializationContext in the derecho namespace, why not declare using mutils::RemoteDeserializationContext instead? In fact, if we were just using pointers to mutils::RemoteDeserializationContext instead of pointers to IDeserializationContext, we wouldn't need that awkward copying for-loop in RPCManager to initialize rdv: rdv is a vector of RemoteDeserializationContext* so we could just copy-construct it from RPCManager's constructor parameter if that was also a vector of RemoteDeserializationContext*.
2. If the vector of IDeserializationContext is supposed to have one IDeserializationContext per type in Group, but they aren't indexed by subgroup type, how will you match a subgroup type to an IDeserializationContext? I think it would probably work to expect this vector to be in the same order as the declaration of type parameters to Group, because that ordering is already used for the subgroup_type_order in ViewManager, but if that's your intention you should document it.

[songweijia]

Thanks to @etremel for the feedback! I changed the deriving with no implementation with using. See commit cd7636c8c6b03002aca9792e956919e32f267710. I want to expose DeserializationContext instead of mutils::RemoteDeserializationContext because the former one is easier to understand for application. And we (de)serialize not only for sending across network, but also for persistence.

For the second question: it is not necessary to have a one-to-one mapping between the deserialization context to subgroup type. The situation could be only some special subgroup need this support. Additionally, the mapping is performed by cast testing, you can check the implementation here: mutils::SerializationSupport::mgr(). We don't need to complicate it with a one-to-one mapping.

[songweijia]

Does anyone want to add more to this? I plan to merge this feature to master.

[etremel]

I think you should go ahead and do it. The new version looks good to me. (I didn't know about mutils::SerializationSupport::mgr(), but with that knowledge the vector makes perfect sense).

[songweijia]

merged to master (commit 283059f)