picongpu
picongpu copied to clipboard
Replace mpl by mp11 (part 1/2)
This PR replaced most uses of boost::mpl
by boost::mp11
. Only boost::mpl::apply
and the corresponding placeholders have not been replaced yet, because there is no direct equivalent in mp11. The functionality of mp11 is directly pulled into the pmacc
namespace, since all mp11 type functions are prefixed with mp_
.
- [x] This PR requires at least Boost 1.66.0, because this is the first Boost version where Boost.Mp11 is available. Separate PR: #3939.
- [x] This PR also makes use of C++17 features, so merging #3949 is a prerequisit.
Looks definitely much shorter.
Taking a time trace of this PR and the dev branch:
dev | this PR | |
---|---|---|
compilation time | 68.1s | 65.4s |
instantiations | 23648 | 16772 |
trace file size | 74.1MiB | 31.3MiB |
I was disappointed that we could not win much in compilation speed. However, the amount of work in the frontend shrunk drastically. The amount of function and class template instantiations where reduced by around a third. This was also very visible in the size of the trace files.
clang frontend trace from dev:
clang frontend trace from this PR:
We can see a great reduction in the green vertical spike lengths, especially during parsing.
Questions to move on:
- How are users and developers expected to access mp11 functionality? Spelling
boost::mp11::mp_....
is quite long. With mpl, we had the namespace aliasbmpl
. So far I have used an aliasbmp
forboost::mp11
in some places. But I think we could also just import mp11 directly intopmacc
andpicongpu
since all metaprogramming functions are prefixed withmp_
. The user could thus spellmp_...
directly in any picongpu or pmacc namespace. What do you think? - Can we move to at least Boost 1.66? Should we split this off and merge it sooner?
- There is no directy equivalent of MPL lambdas in mp11. That is, you cannot define a type function where several parameters are placeholders (e.g.
_1
) and then call something likeapply<TypeFunction, Args>::fn
which replaces the placeholders by actual template parameters. The workaround is to create alias templates and pass higher order type functions. In some cases this is more verbose, but faster to compile (no big impact though) and leads to less long error messages. We could also provide a similar facility on top of mp11. Currently, several places usingmpl::apply
with placeholders remain because they are not easy to transition. We should eventually move away from MPL completely (to not have 2 metaprogramming libraries). Can we merge this PR without finding a solution to MPL lambdas? Do we want to keep MPL for only its lambdas? Do we want a custom solution on top of mp11? - I replaced
mpl::pair
by a plainmp_list
in some cases. For others, I createdpmacc::meta::Pair
. We would not need it though and could just usemp_list
. mp11 does not have a dedicated pair type and this is fine IMO. - Should developers include mp11 headers directly, or should there be a central pmacc header pulling in mp11, that is supposed to be included by everyone else?
- The PR is huge, should/can we split it in parts?
-
pmacc::CT::Vector
could benefit from refactoring to use moreconstexpr
instead of meta programming. The current version (with mp11) is not great, but this is not mp11's fault. Do anything here? - This PR requires C++17 already because of
if constexpr
in some places. Has PIConGPU switched already?
Questions to move on:
- How are users and developers expected to access mp11 functionality? Spelling
boost::mp11::mp_....
is quite long. With mpl, we had the namespace aliasbmpl
. So far I have used an aliasbmp
forboost::mp11
in some places. But I think we could also just import mp11 directly intopmacc
andpicongpu
since all metaprogramming functions are prefixed withmp_
. The user could thus spellmp_...
directly in any picongpu or pmacc namespace. What do you think?
Since pmacc is heavily depending on meta-programming I would be fine to pull mp11 into pmacc.
- Can we move to at least Boost 1.66? Should we split this off and merge it sooner?
Yes would be possible but requires a separate PR. Why do you like to remove boost 1.65.1?
- There is no directy equivalent of MPL lambdas in mp11. That is, you cannot define a type function where several parameters are placeholders (e.g.
_1
) and then call something likeapply<TypeFunction, Args>::fn
which replaces the placeholders by actual template parameters. The workaround is to create alias templates and pass higher order type functions. In some cases this is more verbose, but faster to compile (no big impact though) and leads to less long error messages. We could also provide a similar facility on top of mp11. Currently, several places usingmpl::apply
with placeholders remain because they are not easy to transition. We should eventually move away from MPL completely (to not have 2 metaprogramming libraries). Can we merge this PR without finding a solution to MPL lambdas? Do we want to keep MPL for only its lambdas? Do we want a custom solution on top of mp11?
We can not get rid of compile time lambdas. Most functionality depends on it. I am fine with going on with this PR without removing the mpl lambdas.
- I replaced
mpl::pair
by a plainmp_list
in some cases. For others, I createdpmacc::meta::Pair
. We would not need it though and could just usemp_list
. mp11 does not have a dedicated pair type and this is fine IMO.
I am fine with pmacc::meta::Pair
. Could we maybe use std::tuple
. The code is from c++98 times, nowadays usage of std::tuple should be ok, but maybe I miss something because I have not looked into the usage.
- Should developers include mp11 headers directly, or should there be a central pmacc header pulling in mp11, that is supposed to be included by everyone else?
I think if pmacc is pulling this dependency it is fine.
- The PR is huge, should/can we split it in parts?
If possible I would prefer small PR and changing the parts step by step but since mpl is everywhere used it could be easier to change it within one PR.
pmacc::CT::Vector
could benefit from refactoring to use moreconstexpr
instead of meta programming. The current version (with mp11) is not great, but this is not mp11's fault. Do anything here?
I am fine to use constexpr
where it makes sense and is useful. As I said above most code is from C++98 times and three was no way's around template magic where we could use now more elegant ways and therefore reduce the compile time too.
- This PR requires C++17 already because of
if constexpr
in some places. Has PIConGPU switched already?
No PIConGPU is still C++14. We can switch the dev branch any time to C+17. The last time it was blocked by an issue in the dependency openPMD-api. Switching to C++17 requires a separate PR.
Since pmacc is heavily depending on meta-programming I would be fine to pull mp11 into pmacc.
Done. All functionality is available as pmacc::mp_...
.
Why do you like to remove boost 1.65.1?
Boost.Mp11 is available since Boost 1.66. PR here: #3939
We can not get rid of compile time lambdas. Most functionality depends on it. I am fine with going on with this PR without removing the mpl lambdas.
OK!
I am fine with
pmacc::meta::Pair
. Could we maybe usestd::tuple
. The code is from c++98 times, nowadays usage of std::tuple should be ok, but maybe I miss something because I have not looked into the usage.
pmacc::meta::Pair
is just a type list of fixed length 2. It's literally just:
template<typename First, typename Second>
struct Pair
{
using first = First;
using second = Second;
};
I think if pmacc is pulling this dependency it is fine.
I created a new header pmacc/meta/Mp11.hpp
that pulls in Boost.Mp11.
We can switch the dev branch any time to C+17. The last time it was blocked by an issue in the dependency openPMD-api. Switching to C++17 requires a separate PR.
Can I take this as: Someone else is already working on that?
About MPL Lambdas: I had a closer look on how they work and how this is solved in Mp11. In Mp11, mp_bind
with placeholders is used, producing a quoted metafunction, instead of an MPL placeholder expressions and mpl::apply
. In both cases the result is a type that can be put into type lists or passed as normal template argument.
Now, using mp_bind
on the client side of APIs is probably less elegant. However, with some refactoring we can also eliminate placeholder expression entirely in many cases by passing type functions directly. For example most of meta::ForEach
will be simplified from:
meta::ForEach<FileOutputFields, GetFields<boost::mpl::_1>> ForEachGetFields;
ForEachGetFields(threadParams);
to:
meta::ForEach<FileOutputFields, GetFields> ForEachGetFields;
ForEachGetFields(threadParams);
This is also reflected in Mp11, where many meta functions have a normal version accepting a meta function and a version accepting a quoted meta function (see _q
suffixes).
What is still less elegant is transforming this from MPL:
using UnspecializedSpeciesPlugins = pmacc::mp_list<
plugins::multi::Master<EnergyParticles<boost::mpl::_1>>,
plugins::multi::Master<CalcEmittance<boost::mpl::_1>>,
plugins::multi::Master<BinEnergyParticles<boost::mpl::_1>>,
CountParticles<boost::mpl::_1>,
PngPlugin<Visualisation<boost::mpl::_1, PngCreator>>,
plugins::transitionRadiation::TransitionRadiation<boost::mpl::_1>
>;
With Mp11 bind:
using UnspecializedSpeciesPlugins = pmacc::mp_list<
plugins::multi::Master<pmacc::mp_bind<EnergyParticles, pmacc::_1>>,
plugins::multi::Master<pmacc::mp_bind<CalcEmittance, pmacc::_1>>,
plugins::multi::Master<pmacc::mp_bind<BinEnergyParticles, pmacc::_1>>,
pmacc::mp_bind<CountParticles, pmacc::_1>,
PngPlugin<pmacc::mp_bind<Visualisation, pmacc::_1, PngCreator>>,
pmacc::mp_bind<plugins::transitionRadiation::TransitionRadiation, pmacc::_1>
>;
If we want to support the original placeholder expressions (but using Mp11 placeholders), we could write our own meta function to evaluate it. The implementation is similar to mp_bind
and probably in the order of 50-100 LOCs.
Ultimately, we are going to need to look at the various cases and apply local judgement. But I believe we can solve it and make it look nice :) In a separate PR of course.
We can implement the placeholder expressions ourselves and I am considering that for LLAMA as well now. The implementation is rather simple: https://github.com/alpaka-group/llama/pull/451/files
We can implement the placeholder expressions ourselves and I am considering that for LLAMA as well now. The implementation is rather simple: https://github.com/alpaka-group/llama/pull/451/files
Ohh that's cool. I will check it next year^^
I have this PR on my radar and will try to rebase and test it together with #3983 soon.
I will test this tomorrow on crusher and merge it if all works as expected.
@bernhardmgruber Thanks for your hard work!