Skip to content
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

Reduce unnecessary template instantiations #477

Merged
merged 3 commits into from
Sep 11, 2023

Conversation

tmadlener
Copy link
Collaborator

@tmadlener tmadlener commented Sep 8, 2023

BEGINRELEASENOTES

ENDRELEASENOTES

@wdconinc could you give this a try and see if this solves things already? Currently there are only the components left with the original include. That is mainly because they do not have a .cc file yet, but if necessary we can also introduce that.

@tmadlener
Copy link
Collaborator Author

Failing edm4hep workflow should be fixed with key4hep/EDM4hep#224

@wdconinc
Copy link
Contributor

wdconinc commented Sep 8, 2023

I'll try to fit in a test. I haven't set up my local environments to make it easy to swap out podio without recompiling all of DD4hep...

@tmadlener
Copy link
Collaborator Author

Thanks. In principle this should be visible e.g. when compiling anything that links against edm4hep, right? Maybe I can do some before-/after tests myself to see if I see a difference.

@wdconinc
Copy link
Contributor

wdconinc commented Sep 8, 2023

We've found it to be most noticeable in our reconstruction, where every file includes at least some data model file somewhere. But it also includes dd4hep and acts with EDM4hep, so spack will be wanting to recompile those too...

@wdconinc
Copy link
Contributor

wdconinc commented Sep 8, 2023

Confirmed that this branch removes the expensive json template instantiations. (We're back to being dominated by Eigen template instantiations...)

@tmadlener
Copy link
Collaborator Author

Thank you for the very quick turnaround. Quickly triggering CI again before merging.

@tmadlener tmadlener closed this Sep 11, 2023
@tmadlener tmadlener reopened this Sep 11, 2023
@tmadlener tmadlener merged commit aa8d4b7 into AIDASoft:master Sep 11, 2023
33 of 34 checks passed
@tmadlener tmadlener deleted the fix-json-compile-times branch November 8, 2023 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PODIO_JSON_OUTPUT causes nlohmann/json.hpp in each include file, could be json_fwd.hpp instead
2 participants