Hi, as described in "Genode Foundations", all types which are used as argument or return types in RPC functions must have a default constructor. I do not think that such a limitation is really necessary. So I tried to write a proof of concept [1]. These modifications should allow RPC functions to pass most of the non-default-constructible objects which satisfy the rest of the requirements for RPC argument and return types. I extended the "hello_tutorial" with a little test code to show that it works [2].
Software patterns like RAII [3] are a possible reason for removing that limitation. Quite often classes cannot provide a useful default-constructor which does more than merely setting the object state to "invalid". Because often there are no useful default values. So if a class provides a default-constructor which just sets the object to "invalid", the code complexity increases since care has to be taken that the object is fed with data up to a specific point of time.
Unfortunately, my implementation does not remove default-constructor calls completely: 1. Capabilities still need default-constructors. Msgbuf stores them directly in an array and I did not find the time to check changes of this storage behavior for possible side effects. 2. Output-only _arguments_ have to be default-constructible too. Please note, that non-default-constructible _return_ types are allowed. So this affects only RPC arguments which are explicitly declared as Output-only, for example by adding custom specializations of Rpc_direction. The reason for this limitation is that my Pod_tuple version still stores the RPC arguments directly. While input arguments are copy-constructed, it is hard to copy from an object that does not yet exist.
For the second point, I see no simple solutions: we could create a simple byte buffer instead of a default-constructed output-only argument. Since "normal" RPC arguments have to be trivially copyable [4], a RPC function could easily write to this buffer, which would be accessible by the correctly type-casted reference or pointer. A minimalist version of boost::optional [5] or the like as member of Server_args could make sense. However, it would require more template specializations and case differentiations because the special RPC types Capability and Native_capability are not trivially copyable.
I successfully tested the code with the repositories "hello_tutorial" and "demo" on base-linux. But if you decide to use my code contribution, please note that it is rather a proof of concept. There is room for improvements: * I tried to minimize the code modifications. So some refactoring may be useful: I expanded Meta::Empty by constructors and a typedef, I inserted possibly confusing forward declarations, and Meta::Pod_tuples and Meta::Pod_args could be renamed because they do not only accept POD types [6]. * I completely ignored the trace-related code. I do not know if there is a need of modifications.
Thanks to Johannes Schlatow who gave me the idea of grappling with this topic.
[1] https://github.com/3dik/genode/tree/nodef [2] https://github.com/3dik/genode/commit/c35a8d4a3166da74c46423f5e0b4a35933174f... [3] https://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization [4] http://en.cppreference.com/w/cpp/concept/TriviallyCopyable [5] http://www.boost.org/doc/libs/1_62_0/libs/optional/doc/html/boost_optional/t... [6] http://en.cppreference.com/w/cpp/concept/PODType
Hi Edgar,
thanks for posting this impressive (and unexpected) line of work! As far as I know, this is the first time that someone other than me touched the RPC template code of Genode. ;-)
I especially like the changes of the return-value handling. I was never quite satisfied with this part. Your implementation is much nicer.
Software patterns like RAII [3] are a possible reason for removing that limitation. Quite often classes cannot provide a useful default-constructor which does more than merely setting the object state to "invalid". Because often there are no useful default values. So if a class provides a default-constructor which just sets the object to "invalid", the code complexity increases since care has to be taken that the object is fed with data up to a specific point of time.
I agree.
Unfortunately, my implementation does not remove default-constructor calls completely:
- Capabilities still need default-constructors. Msgbuf stores them
directly in an array and I did not find the time to check changes of this storage behavior for possible side effects.
This is not a problem because a default-constructed 'Capability' has a well-defined 'invalid' state, which is expected (like for the boost::optional types you cited).
- Output-only _arguments_ have to be default-constructible too.
Please note, that non-default-constructible _return_ types are allowed. So this affects only RPC arguments which are explicitly declared as Output-only, for example by adding custom specializations of Rpc_direction. The reason for this limitation is that my Pod_tuple version still stores the RPC arguments directly. While input arguments are copy-constructed, it is hard to copy from an object that does not yet exist.
I doubt that this is a problem in practice. We are not using custom specializations of 'Rpc_direction' at all. If one needs an output-only argument, it should be transferred as return value, which can be a POD struct to carry multiple values. The only use case where an output-only argument would make sense is the delegation of a capability (which cannot by carried in a POD struct) in addition to a return value. But as 'Capability' has a default constructor, this is not an issue.
I successfully tested the code with the repositories "hello_tutorial" and "demo" on base-linux. But if you decide to use my code contribution, please note that it is rather a proof of concept. There is room for improvements:
- I tried to minimize the code modifications. So some refactoring may
be useful:
Thanks. That is good for the review.
I expanded Meta::Empty by constructors and a typedef,
I haven't yet wrapped my head around this change yet. Give me some time to let it sink in.
I inserted possibly confusing forward declarations, and Meta::Pod_tuples and Meta::Pod_args could be renamed because they do not only accept POD types [6].
I agree. With your changes, the (renamed) template should be moved outside of 'meta.h', preferably to 'base/rpc.h'.
- I completely ignored the trace-related code. I do not know if there
is a need of modifications.
Thanks to Johannes Schlatow who gave me the idea of grappling with this topic.
I'd love to hear the backstory about this someday. :-)
For integrating your work upstream, I have just opened the following issue:
https://github.com/genodelabs/genode/issues/2150
Thank you again for submitting your contribution!
Cheers Norman
-
Edgard Schmidt -
Norman Feske