Re: [boost] Review request for variant2, review manager wanted
If anyone would like to step forward to manage the review, please do.
Michael Caisse was the first to graciously offer his services (via Slack) and I'll be glad to have him manage the review. Thanks to Niall who also volunteered. :-) With respect to Niall's request for compile time and run time measurements, I've added a benchmark https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.cpp and the results from compiling and running it on my machine are at https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.md This is not a cherry-picked example that makes variant2 look good, so it loses to std::variant here and there. :-)
On Mon, 25 Feb 2019 at 04:02, Peter Dimov via Boost <boost@lists.boost.org> wrote:
and the results from compiling and running it on my machine are at
https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.md
This is not a cherry-picked example that makes variant2 look good, so it loses to std::variant here and there. :-)
I have added another variant of std::variant by mpark <https://github.com/mpark/variant> and removed boost/variant, as it does not compile (1.69, I guess it needs develop, I fudged mp11 to develop). Run-time results at: https://github.com/degski/variant2/blob/develop/benchmark/benchmark1.md degski -- *"Big boys don't cry" - **Eric Stewart, Graham Gouldman*
https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.md
John Elvind Helset reports (via Slack) that using -DBOOST_VARIANT_VISITATION_UNROLLING_LIMIT=1 with Boost.Variant results in significant runtime improvement on both g++ and Clang, to the point where Boost.Variant consistently outperforms both variant2 and std::variant on this specific benchmark. Looking at the code, it seems to me that the switch statement is always unrolled to the default of 20 even when the variant doesn't have as many alternatives, which causes Clang to refuse to inline the visitation and results in a performance loss.
With respect to Niall's request for compile time and run time measurements, I've added a benchmark
https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.cpp
and the results from compiling and running it on my machine are at
https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.md
This is not a cherry-picked example that makes variant2 look good, so it loses to std::variant here and there. :-)
These are very interesting and useful, thank you. I now have hard evidence with which to persuade people at work that Boost.Variant needs to be purged from our latency critical paths. Thank you. Niall
On Mon, Feb 25, 2019 at 11:59 AM Niall Douglas via Boost <boost@lists.boost.org> wrote:
These are very interesting and useful, thank you. I now have hard evidence with which to persuade people at work that Boost.Variant needs to be purged from our latency critical paths.
Niall if you don't mind, we'd like to know what the largest number of types you have in a variant in that code base is. Thanks
These are very interesting and useful, thank you. I now have hard evidence with which to persuade people at work that Boost.Variant needs to be purged from our latency critical paths.
Niall if you don't mind, we'd like to know what the largest number of types you have in a variant in that code base is.
Somewhere between five and twenty I think, depending on use case. They're scattered all over. For me the main problem is the hidden dynamic memory allocation which is happening every state change inside a critical path. But we don't have C++ 17 available, hence my great interest in variant2. (We actually use a completely renamed fork of Boost, so all macros, headers and namespaces are renamed with a custom prefix to prevent collision with customer Boosts. This takes many hours to regenerate, so we lag latest Boost considerably. Still, if there's a suitably pressing need, it'll get regenerated in order to get Mp11 in order to get variant2) Niall
On Mon, Feb 25, 2019 at 1:13 PM Niall Douglas via Boost <boost@lists.boost.org> wrote:
(We actually use a completely renamed fork of Boost...
Whose idea was this, who set it up, who maintains it, what is different from stock Boost, and what's wrong with using Boost as is (as trillions of other developers already do)? Regards
(We actually use a completely renamed fork of Boost...
Whose idea was this, who set it up, who maintains it, what is different from stock Boost, and what's wrong with using Boost as is (as trillions of other developers already do)?
It's actually not uncommon Vinnie in big megacorps. It's done to avoid the potential for implementation use of Boost symbol colliding with customer use of Boost. Each shop will have some poor sod who has to do the conversion from time to time, and maintain it for internal and sometimes external users. Tony van Eerd was that poor sod during my time at BlackBerry, for example. It'll usually have a bunch of nasty regex doing something like: namespace boost => namespace mycorp_boost_1_30 boost:: => mycorp_boost_1_30:: BOOST_ => MYCORP_BOOST_1_30_ ... They usually rename the boost headers directory too, of course, to mycorp_boost_1_30/*, and all the library blob names. John Maddock maintains a tool which will do some of the renaming for you. But it doesn't rename macros, leaves some stuff in the global namespace due to ADL reasons, and some big megacorp likes to directly use Boost.Config macros to annotate their public functions and/or configure a custom Boost, so they need all the macros renamed too. Anyway, doing all the above is not uncommon. I've seen it a few times in my career now in various clients I've worked for. Usually with some ancient Boost version. And I've seen some big megacorp mix multiple different ancient versions of Boost into a single codebase, too, which is "safe" because nothing can clash. Yay. Niall
On 25. Feb 2019, at 23:14, Niall Douglas via Boost <boost@lists.boost.org> wrote:
John Maddock maintains a tool which will do some of the renaming for you.
Ah, thanks to this I became aware of BCP https://www.boost.org/doc/libs/1_69_0/tools/bcp/doc/html/index.html Best regards, Hans
On Mon, Feb 25, 2019 at 5:14 PM Niall Douglas via Boost <boost@lists.boost.org> wrote:
(We actually use a completely renamed fork of Boost...
Whose idea was this, who set it up, who maintains it, what is different from stock Boost, and what's wrong with using Boost as is (as trillions of other developers already do)?
It's actually not uncommon Vinnie in big megacorps. It's done to avoid the potential for implementation use of Boost symbol colliding with customer use of Boost. Each shop will have some poor sod who has to do the conversion from time to time, and maintain it for internal and sometimes external users. Tony van Eerd was that poor sod during my time at BlackBerry, for example.
Did I rename the namespace? I don't recall. Sounds sad but reasonable.
It'll usually have a bunch of nasty regex doing something like:
namespace boost => namespace mycorp_boost_1_30 boost:: => mycorp_boost_1_30:: BOOST_ => MYCORP_BOOST_1_30_ ...
They usually rename the boost headers directory too, of course, to mycorp_boost_1_30/*, and all the library blob names.
John Maddock maintains a tool which will do some of the renaming for you. But it doesn't rename macros, leaves some stuff in the global namespace due to ADL reasons, and some big megacorp likes to directly use Boost.Config macros to annotate their public functions and/or configure a custom Boost, so they need all the macros renamed too.
Anyway, doing all the above is not uncommon. I've seen it a few times in my career now in various clients I've worked for. Usually with some ancient Boost version. And I've seen some big megacorp mix multiple different ancient versions of Boost into a single codebase, too, which is "safe" because nothing can clash. Yay.
Niall
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas wrote:
https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.md
These are very interesting and useful, thank you. I now have hard evidence with which to persuade people at work that Boost.Variant needs to be purged from our latency critical paths.
Do read my follow-up message as well though. With -DBOOST_VARIANT_VISITATION_UNROLLING_LIMIT=1, boost::variant performs very well. (And even as-is, it also performs well with a more recent g++ (8 or 9) than my 7.4.)
https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.md
These are very interesting and useful, thank you. I now have hard evidence with which to persuade people at work that Boost.Variant needs to be purged from our latency critical paths.
Do read my follow-up message as well though. With -DBOOST_VARIANT_VISITATION_UNROLLING_LIMIT=1, boost::variant performs very well. (And even as-is, it also performs well with a more recent g++ (8 or 9) than my 7.4.)
Oh you've already won a conditional acceptance vote from me no doubt. My sole condition for acceptance is (you guessed it) propagation of triviality of all special functions. I also think after you're into Boost you should submit some more papers to WG21 asking them to change the standard library to match your changes to Boost. std::variant should be your variant2. I don't think anybody on boost-dev would disagree with that. I'll be physically attending the next four WG21 meetings. I intend to be in the room to argue strongly in favour of your papers on this topic. Even if it mildly breaks a small proportion of pre-C++23 code. Niall
Gesendet: Montag, 25. Februar 2019 um 23:05 Uhr
https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.md
These are very interesting and useful, thank you. I now have hard evidence with which to persuade people at work that Boost.Variant needs to be purged from our latency critical paths.
Do read my follow-up message as well though. With -DBOOST_VARIANT_VISITATION_UNROLLING_LIMIT=1, boost::variant performs very well. (And even as-is, it also performs well with a more recent g++ (8 or 9) than my 7.4.)
Oh you've already won a conditional acceptance vote from me no doubt. My sole condition for acceptance is (you guessed it) propagation of triviality of all special functions.
Maybe I nissunderstood your or Peter's mail, but I think the point was that the original boost::variant (not v2) isn't as bad as the initial tests showed if you set -DBOOST_VARIANT_VISITATION_UNROLLING_LIMIT=1. So the hard evidence isn't quite as hard as it seemed. Best Mike
https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.cpp https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark1.md
I added a second benchmark, https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark2.cpp https://github.com/pdimov/variant2/blob/develop/benchmark/benchmark2.md This one is more variant2-friendly, but annoyingly enough, it still doesn't "win" consistently across the board. I should work more on my benchmark-skewing skills.
participants (7)
-
degski -
Gottlob Frege -
Hans Dembinski -
Mike -
Niall Douglas -
Peter Dimov -
Vinnie Falco