-
Notifications
You must be signed in to change notification settings - Fork 157
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
perf!: Reduce Seedfinder heap allocation #905
perf!: Reduce Seedfinder heap allocation #905
Conversation
Codecov Report
@@ Coverage Diff @@
## main #905 +/- ##
=======================================
Coverage 48.66% 48.66%
=======================================
Files 331 331
Lines 17129 17131 +2
Branches 8094 8092 -2
=======================================
+ Hits 8336 8337 +1
Misses 3087 3087
- Partials 5706 5707 +1
Continue to review full report at Codecov.
|
namespace ActsExamples { | ||
|
||
/// A proto track is a collection of hits identified by their indices. | ||
using ProtoTrack = std::vector<Index>; | ||
using ProtoTrack = boost::container::small_vector<Index, 10>; |
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.
this uses 3x more space than necessary for 3 SP, right?
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.
Yeah true, I just went with a low-ish number. Let's see what I get with 3.
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.
In my testing, reducing to 3 appears to actually slow things down by about 3%, which I can't explain. I suggest we keep it at 10.
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.
The keyword for the changed interface needs to be in the PR description, right?
Added. |
Clang seems to dislike the template template. |
Should compile now. |
This is just a tiny little improvement on acts-project#905. I noticed that we are currently using an output iterator function to write some results to a vector, and then copying that vector over to append it to another, existing vector. I think it would be (very marginally) quicker and neater to avoid this extra copy and just have the function take the insertion iterator of the vector we are planning to write to in the end.
This is just a tiny little improvement on acts-project#905. I noticed that we are currently using an output iterator function to write some results to a vector, and then copying that vector over to append it to another, existing vector. I think it would be (very marginally) quicker and neater to avoid this extra copy and just have the function take the insertion iterator of the vector we are planning to write to in the end.
This is just a tiny little improvement on #905. I noticed that we are currently using an output iterator function to write some results to a vector, and then copying that vector over to append it to another, existing vector. I think it would be (very marginally) quicker and neater to avoid this extra copy and just have the function take the insertion iterator of the vector we are planning to write to in the end.
The Seed finder does a large number of heap allocations for various different things. With this PR, I'm reducing the number of heap allocations considerably with little change to the public interface.
On my machine, the runtime of the
SeedingAlgorithm
on 10000 events with the generic detector drops from about 3s to 2s, for about a 33% speedup.The number of heap allocations is reduced from 220 million to just over 300000 so about a reduction of >99%.
I checked that the estimated parameter ROOT files are identical by using our
compareRootFiles.C
script.BREAKING CHANGE: The interface of a number of methods changes:
SeedFilter::filterSeeds_2SpFixed
(takes an output iterator, is now void)SeedFilter::filterSeeds_1SpFixed
(takes an output iterator, is now void)Seedfinder::createSeedsForGroup
(takes an output iterator, is now void, takes a state object as a reference)Grid::collect
(returnsboost::container::small_vector<size_t, N>
whereN = 3^DIM
of the grid)