-
Notifications
You must be signed in to change notification settings - Fork 27
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
FM refactoring and compile times reduction #163
Conversation
Codecov Report
@@ Coverage Diff @@
## master #163 +/- ##
==========================================
+ Coverage 78.32% 79.21% +0.89%
==========================================
Files 201 203 +2
Lines 19530 19736 +206
Branches 7995 7988 -7
==========================================
+ Hits 15296 15634 +338
+ Misses 4234 4102 -132
... and 59 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Only minor comments.
@@ -45,12 +45,12 @@ | |||
|
|||
namespace mt_kahypar { | |||
|
|||
template<typename TypeTraits, typename GainTypes> | |||
template<typename CombinedTraits> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this change. Can we rethink the name CombinedTraits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure. Any suggestions? Maybe GraphAndGainTypes
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT about also removing the FM stats? We're way past using them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea 👍
using KWayPriorityQueue = kahypar::ds::KWayPriorityQueue<HypernodeID, Gain, std::numeric_limits<Gain>, false>; | ||
using ThreadLocalKWayPriorityQueue = tbb::enumerable_thread_specific<KWayPriorityQueue>; | ||
|
||
using ThreadLocalFastResetFlagArray = tbb::enumerable_thread_specific<kahypar::ds::FastResetFlagArray<> >; | ||
|
||
using InitialPartitionerFactory = kahypar::meta::Factory<InitialPartitioningAlgorithm, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what the best policy is, but let's discuss. Factories close to the definitions or factories in a dedicated factory header? We should try to be consistent, i.e., the refiner and coarsener factories are in a dedicated header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. I guess it would make sense to move InitialPartitionerFactory
also to factories.h
using DeltaPartitionedHypergraph = typename PartitionedHypergraph::template DeltaPartition<DeltaGainCache::requires_connectivity_set>; | ||
using AttributedGains = typename GainTypes::AttributedGains; | ||
using AttributedGains = typename CombinedTraits::AttributedGains; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think AttributedGains can be removed.
…e compilation time
~7% overall speedup with all features
~14% overall speedup with all features
marginal improvement at best
…gorithms This splits the long compile time of pool_initial_partitioner in two
…lass reduces compile time of register_initial_partitioning_algorithms significantly
~10% improvement for clean builds
localized_kway_fm_core
to reduce compile times and remove code that is not used by our configurations (also introduces a small optimization for graphs)Clean build times are ~1.5 times faster on my machine while the compilation time for the slowest files is reduced by a factor of 2-3.