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.

Sign up for GitHub

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

Reduce the symbol load caused by feature templates #41

Merged
merged 9 commits into from
May 7, 2020
Merged

Reduce the symbol load caused by feature templates #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
027533a
Remove the use of the aggregator
mxgrey Apr 7, 2020
f42517c
Use flat and lean inheritance structure
mxgrey Apr 8, 2020
0c0103a
Remove all unnecessary template instantiations
mxgrey Apr 8, 2020
b4815cc
Remove unnecessary code
mxgrey Apr 8, 2020
e8f0b76
Fix code check
mxgrey Apr 9, 2020
d3755b9
Add a unit test for FlattenFeatures
mxgrey Apr 9, 2020
346ee84
Make a template easier for MSVC
mxgrey Apr 13, 2020
5063964
Merge branch 'ign-physics2' into reduce_symbols
scpeters Apr 30, 2020
f0d33b1
Merge branch 'ign-physics2' into reduce_symbols
chapulina May 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion dartsim/src/plugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ struct DartsimFeatures : FeatureList<
> { };

class Plugin :
public virtual Implements3d<DartsimFeatures>,
public virtual Base,
public virtual CustomFeatures,
public virtual EntityManagementFeatures,
Expand Down
224 changes: 200 additions & 24 deletions include/ignition/physics/detail/FeatureList.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ namespace ignition
class ExtractFeatures
: public VerifyFeatures<F>
{
public: using Result = std::tuple<F>;
public: using type = std::tuple<F>;
};

/// \private This specialization of ExtractFeatures is used to wipe away
Expand All @@ -182,7 +182,7 @@ namespace ignition
class ExtractFeatures<std::tuple<F...>>
: public VerifyFeatures<F...>
{
public: using Result = std::tuple<F...>;
public: using type = std::tuple<F...>;
};

/// \private This specialization of ExtractFeatures is used to wipe away
Expand All @@ -195,7 +195,7 @@ namespace ignition
void_t<typename SomeFeatureList::Features>>
: public VerifyFeatures<typename SomeFeatureList::Features>
{
public: using Result = typename SomeFeatureList::Features;
public: using type = typename SomeFeatureList::Features;
};

/// \private This specialization skips over any void entries. This allows
Expand All @@ -204,22 +204,35 @@ namespace ignition
template <>
class ExtractFeatures<void>
{
public: using Result = std::tuple<>;
public: using type = std::tuple<>;
};

/////////////////////////////////////////////////
template <typename DiscardTuple, typename InputTuple>
/// \private This struct wraps the TupleContainsBase class to create a
/// tuple filter that can be passed to FilterTuple.
template <typename DiscardTuple>
struct RedundantTupleFilter
{
template <typename T>
struct Apply : TupleContainsBase<T, DiscardTuple> { };
};

/////////////////////////////////////////////////
template <template <typename> class Filter, typename InputTuple>
struct FilterTuple;

/////////////////////////////////////////////////
template <typename DiscardTuple, typename... InputTypes>
struct FilterTuple<DiscardTuple, std::tuple<InputTypes...>>
/// \private This class will apply a Filter to a tuple and produce a new
/// tuple that only includes the tuple elements which were ignored by the
/// Filter.
template <template <typename> class Filter, typename... InputTypes>
struct FilterTuple<Filter, std::tuple<InputTypes...>>
{
using Result = decltype(std::tuple_cat(
using type = decltype(std::tuple_cat(
std::conditional_t<
// If the input type is a base class of anything that should be
// discared ...
TupleContainsBase<InputTypes, DiscardTuple>::value,
Filter<InputTypes>::value,
// ... then we should leave it out of the final tuple ...
std::tuple<>,
// ... otherwise, include it.
Expand All @@ -228,6 +241,129 @@ namespace ignition
>()...));
};

/////////////////////////////////////////////////
/// \private Use this struct to remove the tuple elements of one tuple
/// from another tuple
template <typename DiscardTuple>
struct SubtractTuple
{
template <typename T>
using Filter =
typename RedundantTupleFilter<DiscardTuple>::template Apply<T>;

/// This struct will contain a nested struct called `type` which will be
/// a tuple with all the elements of FromTuple that are not present in
/// DiscardTuple
template <typename FromTuple>
struct From : FilterTuple<Filter, FromTuple> { };
};

/////////////////////////////////////////////////
/// \private This template takes in a tuple as InputTuple and gives back
/// a tuple where every element type is only present once.
template <typename InputTuple>
struct RemoveTupleRedundancies;

template <typename T>
struct wrap { };

template <typename Tuple>
struct unwrap;

template <typename... T>
struct unwrap<std::tuple<wrap<T>...>>
{
using type = std::tuple<T...>;
};

template <typename... InputTupleArgs>
struct RemoveTupleRedundancies<std::tuple<InputTupleArgs...>>
{
template <typename PartialResultInput, typename...>
struct Impl;

template <typename PartialResultInput>
struct Impl<PartialResultInput>
{
using type = std::tuple<>;
};

template <typename ParentResultInput, typename F1, typename... Others>
struct Impl<ParentResultInput, F1, Others...>
{
using PartialResult =
std::conditional_t<
TupleContainsBase<wrap<F1>, ParentResultInput>::value,
std::tuple<>,
std::tuple<wrap<F1>>
>;

using AggregateResult = decltype(std::tuple_cat(
ParentResultInput(), PartialResult()));

using type = decltype(std::tuple_cat(
PartialResult(),
typename Impl<AggregateResult, Others...>::type()));
};

using type =
typename unwrap<
typename Impl<std::tuple<>, InputTupleArgs...>::type
>::type;
};

/////////////////////////////////////////////////
/// \private This template is used to take a hierarchy of FeatureLists and
/// flatten them into a single tuple.
template <typename FeatureTuple, typename = void_t<>>
struct FlattenFeatures;

/////////////////////////////////////////////////
/// \private This template is a helper for FlattenFeatures
template <typename FeatureOrList, typename = void_t<>>
struct ExpandFeatures
{
using type = std::conditional_t<
std::is_void_v<typename FeatureOrList::RequiredFeatures>,
std::tuple<FeatureOrList>,
decltype(std::tuple_cat(
std::tuple<FeatureOrList>(),
typename FlattenFeatures<
typename FeatureOrList::RequiredFeatures>::type()))
>;
};

/////////////////////////////////////////////////
template <typename List>
struct ExpandFeatures<List, void_t<typename List::Features>>
{
using type = typename FlattenFeatures<typename List::Features>::type;
};

/////////////////////////////////////////////////
template <typename FeatureListT>
struct FlattenFeatures<
FeatureListT, void_t<typename FeatureListT::FeatureTuple>>
{
using type =
typename FlattenFeatures<typename FeatureListT::FeatureTuple>::type;
};

/////////////////////////////////////////////////
template <typename... Features>
struct FlattenFeatures<std::tuple<Features...>, void_t<>>
{
using type = decltype(std::tuple_cat(
typename ExpandFeatures<Features>::type()...));
};

/////////////////////////////////////////////////
template <>
struct FlattenFeatures<void>
{
using type = std::tuple<>;
};

/////////////////////////////////////////////////
/// \private CombineListsImpl provides the implementation of CombineLists.
/// This helper implementation structure allows us to filter out repeated
Expand All @@ -238,7 +374,7 @@ namespace ignition
template <typename PartialResultInput>
struct CombineListsImpl<PartialResultInput>
{
using Result = std::tuple<>;
using type = std::tuple<>;
};

template <typename ParentResultInput, typename F1, typename... Others>
Expand All @@ -247,28 +383,27 @@ namespace ignition
// Add the features of the feature list F1, while filtering out any
// repeated features.
using InitialResult =
typename FilterTuple<
ParentResultInput,
typename ExtractFeatures<F1>::Result
>::Result;
typename SubtractTuple<ParentResultInput>
::template From<typename ExtractFeatures<F1>::type>::type;

// Add the features that are required by F1, while filtering out any
// repeated features.
using PartialResult = decltype(std::tuple_cat(
InitialResult(),
typename FilterTuple<
decltype(std::tuple_cat(ParentResultInput(), InitialResult())),
typename ExtractFeatures<typename F1::RequiredFeatures>::Result
>::Result()));
typename SubtractTuple<
decltype(std::tuple_cat(ParentResultInput(), InitialResult()))>
::template From<
typename ExtractFeatures<typename F1::RequiredFeatures>::type
>::type()));

// Define the tuple that the child should use to filter its list
using ChildFilter =
decltype(std::tuple_cat(ParentResultInput(), PartialResult()));

// Construct the final result
using Result = decltype(std::tuple_cat(
using type = decltype(std::tuple_cat(
PartialResult(),
typename CombineListsImpl<ChildFilter, Others...>::Result()));
typename CombineListsImpl<ChildFilter, Others...>::type()));
};

/// \private CombineLists is used to take variadic lists of features,
Expand All @@ -280,7 +415,7 @@ namespace ignition
struct CombineLists
{
public: using Result =
typename CombineListsImpl<std::tuple<>, FeatureLists...>::Result;
typename CombineListsImpl<std::tuple<>, FeatureLists...>::type;
};

/////////////////////////////////////////////////
Expand Down Expand Up @@ -367,9 +502,50 @@ namespace ignition
template <template<typename> class Selector, typename FeatureListT>
struct ExtractAPI
{
public: template<typename... T>
class type : public virtual
Aggregate<Selector, FeatureListT>::template type<T...> { };
template <typename FeatureTuple, typename... T>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the Aggregate template be removed now that ExtractAPI doesn't use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review, @azeey ! I'll go ahead and merge this PR to unblock ign-gazebo, but @mxgrey , feel free to follow up in a subsequent PR if needed.

Copy link
Contributor

@mxgrey mxgrey May 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell, this still checks for duplicate Features, but I remember you mentioned that in order to reduce compile times, we have to assume that there are no duplicate Features in a FeatureList. Is that still the case?

That is not the case, fortunately! There should be no problem at all with having duplicate features.

Can ComposePlugin be updated to work like the new ExtractAPI? Would that save any more compile time?

I considered that, but I didn't have enough time to really explore it. When I inspect the symbols that get generated, there are relatively few symbols related to ComposePlugin, most likely because all entity types for the same set of features share the same plugin type, so cutting down on ComposePlugin probably won't offer much. It's certainly worth trying if someone has cycles to spare.

Can the Aggregate template be removed now that ExtractAPI doesn't use it?

For some reason I thought Aggregate was still being used somewhere, but I can no longer find wherever I thought it was being used. So sure, I guess we can throw it out now.

struct Select;

template <typename... Features, typename... T>
struct Select<std::tuple<Features...>, T...>
{
using type =
typename RemoveTupleRedundancies<
std::tuple<typename Selector<Features>::template type<T...>...>
>::type;
};

template <typename T>
struct Filter : std::is_same<Selector<T>, Empty> { };

template <typename... T>
using Bases =
typename Select<
typename FilterTuple<
Filter,
typename FlattenFeatures<FeatureListT>::type
>::type,
T...
>::type;

template <typename... T>
struct S : std::tuple_size<Bases<T...>> { };

template <typename... T>
using IndexSequence = std::make_index_sequence<S<T...>::value>;

template <typename>
struct Impl;

template <std::size_t... I>
struct Impl<std::index_sequence<I...>>
{
template<typename... T>
class type
: public virtual std::tuple_element<I, Bases<T...>>::type... { };
};

template <typename... T>
using type = typename Impl<IndexSequence<T...>>::template type<T...>;
};
}

Expand Down
7 changes: 7 additions & 0 deletions src/FeatureList_TEST.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,13 @@ TEST(FeatureList_TEST, Hierarchy)
// This test makes sure that a hierarchy of FeatureLists can compile.
// As long as the line below can compile, the test is passed.
HierarchyLevel3();

using Level3Tuple = detail::FlattenFeatures<HierarchyLevel3>::type;
EXPECT_TRUE((detail::TupleContainsBase<FeatureA, Level3Tuple>::value));
EXPECT_TRUE((detail::TupleContainsBase<FeatureB, Level3Tuple>::value));
EXPECT_TRUE((detail::TupleContainsBase<FeatureC, Level3Tuple>::value));
EXPECT_TRUE((detail::TupleContainsBase<Conflict1, Level3Tuple>::value));
EXPECT_TRUE((detail::TupleContainsBase<Conflict2, Level3Tuple>::value));
}

int main(int argc, char **argv)
Expand Down
7 changes: 4 additions & 3 deletions src/FilterTuple_TEST.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ class ClassD : public ClassB { };
using Filter = std::tuple<ClassA, ClassD>;
using Input = std::tuple<ClassB, ClassC, ClassD, ClassA>;

using Result = ignition::physics::detail::FilterTuple<Filter, Input>::Result;
using Result = ignition::physics::detail::SubtractTuple<Filter>
::From<Input>::type;

using UnfilteredResult =
ignition::physics::detail::FilterTuple<std::tuple<>, Input>::Result;
ignition::physics::detail::SubtractTuple<std::tuple<>>::From<Input>::type;

TEST(FilterTuple_TEST, FilterTupleResult)
{
Expand All @@ -46,7 +47,7 @@ TEST(FilterTuple_TEST, FilterTupleResult)

using SingleCombineLists =
ignition::physics::detail::CombineListsImpl<
std::tuple<>, ClassA>::Result;
std::tuple<>, ClassA>::type;

using SingleCombineListsInitial =
ignition::physics::detail::CombineListsImpl<
Expand Down