acts-project
refactor: Do not allocate memory if the surface is connected with a detector el…
If the surface has an associated detector element its internal transform is never accessed consuming memory for no-reason. If the transform is a unique_ptr which is released in such cases, the footprint should be reduced
Codecov Report
Attention: Patch coverage is 23.52941% with 13 lines in your changes missing coverage. Please review.
Project coverage is 47.64%. Comparing base (
5885ee8) to head (3939692).
Additional details and impacted files
@@ Coverage Diff @@
## main #3069 +/- ##
==========================================
- Coverage 47.65% 47.64% -0.01%
==========================================
Files 507 507
Lines 29205 29211 +6
Branches 14010 14012 +2
==========================================
+ Hits 13917 13919 +2
- Misses 5264 5265 +1
- Partials 10024 10027 +3
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
📊: Physics performance monitoring for d49e825e09f7f8f26c63cb7749a2435471df0f92
physmon summary
- ✅ CKF truth_smeared
- ✅ IVF truth_smeared
- ✅ AMVF truth_smeared
- ✅ Track Summary CKF truth_smeared
- ✅ Seeding truth_estimated
- ✅ CKF truth_estimated
- ✅ IVF truth_estimated
- ✅ AMVF truth_estimated
- ✅ Track Summary CKF truth_estimated
- ✅ Seeding seeded
- ✅ CKF seeded
- ✅ IVF seeded
- ✅ AMVF seeded
- ✅ AMVF (+grid seeder) seeded
- ✅ Track Summary CKF seeded
- ✅ Seeding orthogonal
- ✅ CKF orthogonal
- ✅ IVF orthogonal
- ✅ AMVF orthogonal
- ✅ Track Summary CKF orthogonal
- ✅ Ambisolver seeded
- ✅ Ambisolver orthogonal
- ✅ Seeding ttbar
- ✅ CKF ttbar
- ✅ Ambisolver
- ✅ Track Summary CKF ttbar
- ✅ AMVF ttbar
- ✅ AMVF (+grid seeder) ttbar
- ✅ Truth tracking (GSF)
- ✅ Truth tracking
- ✅ Truth tracking (GX2F)
- ✅ Particles fatras
- ✅ Particles geant4
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
This is what we used to have before. I think @asalzburger at some point measured that having the transform locally improved performance. But I forget the details.
Hmm, the transform is never used if the detector element is present.... Confused, I am
The transform is only used for surfaces which don't have a detector element. Otherwise the transform is retrieved from the detector element, and that's where for example the alignment is handled.
So for all other surfaces, boundaries etc, the transform is accessed locally.
This is what we used to have before. I think @asalzburger at some point measured that having the transform locally improved performance. But I forget the details.
Hi, yes I remember that there was a visible (but small performance change), however, we can just see with this PR if this is still a measurable effect?
I could imagine that this is an additional cache miss because of the additional indirection. After measuring this we would have to weight the CPU increase against the MEM decrease
Results from the propagation test on 3 threads
patched:
10:03:28 Sequencer INFO Processed 100000 events in 1223.381458 s (wall clock)
10:03:28 Sequencer INFO Average time per event: 48.693846 ms/event
main:
22:24:31 Sequencer INFO Processed 100000 events in 1237.661429 s (wall clock)
22:24:31 Sequencer INFO Average time per event: 49.130005 ms/event
@junggjo9 @paulgessinger @asalzburger @andiwand In the tests, it makes a difference of around 1% (not sure if this is significant). How should we proceed with this?
There are a couple of CI failure
- Can you check the formatting @junggjo9 ?
- I guess the
CI Bridgeones are false positives - codecov complains - I guess because there are a couple more branches which are not directly tested right now? Should we add some unit tests to capture the new behavior?
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
68.0% Coverage on New Code
0.0% Duplication on New Code
See analysis details on SonarCloud
![sonarqubecloud[bot] avatar](https://avatars.githubusercontent.com/in/12526?v=4 "Published by a pub.dev verified publisher"){kind=small}
