Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

DPL Analysis: avoid instantiating HistFactory functions for each TU#6302

Merged
ktf merged 2 commits intoAliceO2Group:devAliceO2Group/AliceO2:devfrom
mario-krueger:devmario-krueger/AliceO2:devCopy head branch name to clipboard
Jun 2, 2021
Merged

DPL Analysis: avoid instantiating HistFactory functions for each TU#6302
ktf merged 2 commits intoAliceO2Group:devAliceO2Group/AliceO2:devfrom
mario-krueger:devmario-krueger/AliceO2:devCopy head branch name to clipboard

Conversation

@mario-krueger
Copy link
Member

  • move histogram creation code to HistogramSpec.cxx
  • instantiate histogram creation templates only once

- move histogram creation code to HistogramSpec.cxx
- instantiate histogram creation templates only once
@mario-krueger mario-krueger requested review from a team, iarsene and jgrosseo as code owners June 2, 2021 12:41
@mario-krueger
Copy link
Member Author

This is only a first cleanup step to improve compile time and memory related to the HistogramRegistry.
More changes will be needed that directly affect the registry code, but I'd prefer to do this incrementally.

@ktf
Copy link
Member

ktf commented Jun 2, 2021

Thank you for this. You can check how long a given file takes to compile by using:

scripts/profile-compilation ./Analysis/Tasks/PWGHF/taskXic.cxx

There is also a script which is called scripts/profile-compilation-merge which allows you to combine more then one compilation unit. You can look at the results with https://speedscope.app.

That said when I try to do a full compilation with clang and -ftime-trace, using https://github.com/aras-p/ClangBuildAnalyzer to do a full report, I still see roughly the same compilation time.

@mario-krueger
Copy link
Member Author

Thanks for the hints! That will be very helpful.
You are right, I also just realised these changes will not have much effect on the compilation since the templated function (createHist<>) which I extern'ed here was instanciated only in the HistogramRegistry.cxx itself and in the CorrelationContainer, but never in the user tasks. However, I still think these are useful changes if in the future people want to create histograms in the same manner as Jan Fiete does it in the CorrelationContainer.
The next modifications to the registry will probably have a larger impact.

@ktf
Copy link
Member

ktf commented Jun 2, 2021

Yes, agreed.

@ktf
Copy link
Member

ktf commented Jun 2, 2021

Errors unrelated. macOS tested on my own box.

@ktf ktf merged commit e3e33a6 into AliceO2Group:dev Jun 2, 2021
cortesep pushed a commit to cortesep/AliceO2 that referenced this pull request Jun 11, 2021
…liceO2Group#6302)

- move histogram creation code to HistogramSpec.cxx
- instantiate histogram creation templates only once
EmilGorm pushed a commit to EmilGorm/AliceO2 that referenced this pull request Nov 22, 2021
…liceO2Group#6302)

- move histogram creation code to HistogramSpec.cxx
- instantiate histogram creation templates only once
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.