eggs-cpp / variant

Eggs.Variant is a C++11/14/17 generic, type-safe, discriminated union.
http://eggs-cpp.github.io/variant/
Boost Software License 1.0
137 stars 27 forks source link

Visiting eggs::variant differently reduce seriously binary size #19

Closed daminetreg closed 8 years ago

daminetreg commented 8 years ago

Hi @K-ballo,

I'm currently optimizing a code which uses heavily boost::variant at insane sizes. Switching to eggs cpp is a great thing for us, even though eggs cpp is not yet officially released, because it allows us to have an endless variant, it scales better than increasing mpl sizes ( i.e. no need for custom -DBOOST_MPL_LIMIT_LIST_SIZE=XXX as with Boost.Variant ).

I've been looking into it, and when I take eggs::variants::apply function it looks like it's recursing to unpack the variadic parameter pack. And therefore I get a binary which is of 868Kb, when I reimplement the apply function as below, using another way to iterate over variadic parameter pack from Joël Falcou (c.f. https://www.youtube.com/watch?feature=player_detailpage&v=idPThkw2p6c#t=1984), I get a binary of size 784Kb. Because there is way less symbols generated by the variant visitation, and in our code we visit variants often.

namespace pre { namespace variant { 

  template<typename... Ts>
  using pack_params = eggs::variants::detail::pack< eggs::variants::detail::empty, typename std::remove_cv<Ts>::type...  >;

  template<typename F, typename... Ts, typename std::enable_if<!std::is_same<typename F::result_type, void>::value>::type* = nullptr>
  auto apply(F f, const eggs::variant<Ts...>& variant) -> typename F::result_type {
    typename F::result_type result;
    return std::initializer_list<int>{( (variant.which() == eggs::variants::detail::index_of<Ts, pack_params<Ts...>>::value - 1) 
        ? 
        (result = std::ref(f)(eggs::variants::get<Ts>(variant))),0 : 0)...}, result;

  }

  template<typename F, typename... Ts, typename std::enable_if<std::is_same<typename F::result_type, void>::value>::type* = nullptr>
  auto apply(F f, const eggs::variant<Ts...>& variant) -> void {
    std::initializer_list<int>{( (variant.which() == eggs::variants::detail::index_of<Ts, pack_params<Ts...>>::value - 1) 
        ? 
        (std::ref(f)(eggs::variants::get<Ts>(variant))),0 : 0)...};

  }

}}

I think you have a reason why you implemented the visitation by recursing over the parameter pack, therefore I'm asking you if you were open to a PR implementing visitation this way ?

Naturally I would polish it first, and change the result-type to a functions-traits one.

Or perhaps there is something I don't get right ? I must admit I read through really fast your code, so I may have misunderstood or missed something. But from my analysis and understanding apply would benefit from implementing it this way.

Cheers, :smile:

K-ballo commented 8 years ago

Hi @daminetreg,

The recursion in apply is there to support multi-visitation, which your snippet does not seem to cover. Recursion happens only once per variant, not once per variant element, so in the simple visitation case there should be no recursion involved.

Perhaps you could provide me with a test case so I can see if there's any obvious overheads in the multi visitation code for the simple visitation case. It might be possible to simplify visitation for the common case, and given your results it seems it would be worth it too.

That said, I am not entirely happy with the way multi-visitation is currently implemented. The approach in mpark/variant looks much more promising, and I've been considering switching since I've read the article. I'd be interested in seeing how it behaves for your test case too.

daminetreg commented 8 years ago

Hi @K-ballo,

So if you don't have any recursion for the simple visitation, it is strange indeed. I'll recheck and give a sample to show the differences.

mpark/variant looks good but is C++14 only, and even though we will switch soon to C++14, I cannot for the moment and still need C++11 support, but with the examples I'll build the existing code I can then try as well with the mpark implementation.

I'll come back as soon as I have a test code for the case where we can see the size differences between with eggs::variants::apply and my snippets above.

K-ballo commented 8 years ago

It's possible that the binary size difference comes from any of the return type deduction traits, the INVOKE mechanism, or the multi-visitation scaffolding. I will reopen and analyze the situation once we have a driving example.