Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explain the rationale for msgpack::object as the API type #86

Open
oberstet opened this issue Jan 31, 2016 · 2 comments
Open

Explain the rationale for msgpack::object as the API type #86

oberstet opened this issue Jan 31, 2016 · 2 comments
Labels

Comments

@oberstet
Copy link
Contributor

Initially, we used boost::any as the API type. We have switched to msgpack::object for good reasons. Since this is a critical API design aspect, it would make sense to explain "the good reasons".

@jpetso
Copy link
Contributor

jpetso commented Jan 31, 2016

As far as I'm concerned, these were the main points:

  • boost::any required RTTI. I think Boost now offers a way to provide static typeid functions and make it work without RTTI, but even then this kind of cast will be slow because it involves (typeid) string comparisons, and providing the static typeid functions in addition to msgpack/JSON/CBOR conversion functions adds unnecessary extra complexity.
  • boost::any seems convenient but was actually pretty sucky, because you only supported a small number of types for packing. This means if a user has their own custom type, they would have to convert that manually to a boost::any type that doesn't use their own type but instead contains std::map, std::vector or whatever is supported by autobahn-cpp. Worse, they can assign their own types to boost::any but only get the check at runtime when autobahn-cpp throws an error that it doesn't know how to handle it. With msgpack conversion functions, you know you'll be able to send it if it compiles.
  • Any type will require conversion functions from and to the serialized format. By using msgpack's own conversion system (which serializes any type with a packing template defined, and deserializes into msgpack::object), we allow people to reuse msgpack-c conversions for autobahn-cpp, and the other way round. I believe that's better than defining your own set of conversion functions that users will always have to implement (in addition to msgpack's or any other ones).
    • Note also that msgpack::object is only used when receiving messages. On sending them, we go directly from to serialized msgpack buffers, with no intermediate variant type. For packing non-msgpack serialization formats like JSON, we would have to either go through an intermediate variant (e.g. msgpack::object) or autobahn-cpp / the JSON library offers similar SAX-style conversion functions for direct-to-buffer packing.
  • msgpack::object is the result of msgpack-c's unpack() function. If you convert to any other type, you (currently) have to deserialize using buffer => msgpack::object => (user type). If you choose any other variant type, you have to do it like, buffer => msgpack::object => (autobahn-cpp variant) => (user type). Whether you use boost::any, or msgpack::type::variant, or autobahn::my_custom_variant, there's always an extra step of allocations compared to the msgpack::object baseline.
    • This point is fixable with additional msgpack-c APIs, and that's why I asked for a SAX-style unpacking API in the msgpack-c issue.
      • Even if you have those additional APIs, the only way to gain efficiencies would be to get rid of variant types altogether, and have the user call e.g. session->call<MyType>(...), with the template argument specifying the return type that autobahn-cpp will use to unpack the msgpack buffer.
  • msgpack::object minimizes the number of heap allocations by relying on its msgpack::zone. That makes the API sort of clunky, but is pretty okay for performance compared to an intermediate variant type that uses std::string, std::map, std::vector etc., assuming that variant format is not the final type that the autobahn-cpp user uses in their code.

@oberstet
Copy link
Contributor Author

oberstet commented Feb 1, 2016

@jpetso Thanks a lot for this thorough summary! I think this is a list of very good reasons;) Given that, I feel it was the right decision, and eventually, above should go into the library documentation so users can follow the arguments and decisions we made regarding API (or disagree .. but then only after considering all aspects we had in mind).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants