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
Phase 2 tracker: template CPE #28448
Changes from 8 commits
aeffc45
b79d8a4
26ec5aa
586c895
c1d69b4
73abc0b
be8d81c
33f3e7c
ef2998b
a4ea534
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -207,12 +207,12 @@ struct SiPixelTemplateStore { //!< template storage structure | |||||||
SiPixelTemplateHeader head; | ||||||||
#ifndef SI_PIXEL_TEMPLATE_USE_BOOST | ||||||||
float cotbetaY[60]; | ||||||||
float cotbetaX[5]; | ||||||||
float cotalphaX[29]; | ||||||||
float cotbetaX[60]; | ||||||||
float cotalphaX[60]; | ||||||||
//!< 60 Barrel y templates spanning cluster lengths from 0px to +18px [28 entries for fpix] | ||||||||
SiPixelTemplateEntry enty[60]; | ||||||||
//!< 29 Barrel x templates spanning cluster lengths from -6px (-1.125Rad) to +6px (+1.125Rad) in each of 5 slices [3x29 for fpix] | ||||||||
SiPixelTemplateEntry entx[5][29]; | ||||||||
SiPixelTemplateEntry entx[60][60]; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the impact on memory? (my quick estimate would indicate that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @makortel do you have a specific memory check you would like to see? I am not sure if I ran the time and memory module comparison here:
After PR:
I also ran the profile igprof, this shows the CPU use: And this shows the memory use (but I can't really tell anything from these logs so maybe you are more familiar navigating them): There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Mainly if I estimated the object size correctly or not.
According to dxr
which I suppose is fine as that code path is not run within CMSSW. And I see the corresponding piece in the code path run within CMSSW cmssw/CondFormats/SiPixelTransient/src/SiPixelTemplate.cc Lines 734 to 735 in 9f9e834
was already change to heap allocation in #22458. Good, sorry for the noise. |
||||||||
void destroy(){}; | ||||||||
#else | ||||||||
float* cotbetaY = nullptr; | ||||||||
|
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.
One should update also the comment with the modified dimensions
Even better if the number of templates is saved into a named constant, that can be addressed from whenever needed (therefore, one could define it in SiPixelTemplate.h, or in SiPixelTemplateDefs.h, and access it here and whenever else is needed)
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.
@perrotta Ok, I implemented your suggested changes here: ef2998b