Optimize include time, push definitions to object files#1774
Optimize include time, push definitions to object files#1774ax3l merged 23 commits intoopenPMD:devfrom
Conversation
franzpoeschel
added
discussion
refactoring
api: breaking
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| template <typename Visitor, typename... Args> | ||
| auto RecordComponent::visit(Args &&...args) | ||
| inline auto RecordComponent::visit(Args &&...args) |
Check notice
Code scanning / CodeQL
Unused static variable Note
|
|
||
| template <typename Visitor, typename... Args> | ||
| auto RecordComponent::visit(Args &&...args) | ||
| inline auto RecordComponent::visit(Args &&...args) |
Check notice
Code scanning / CodeQL
Unused local variable Note
| } | ||
|
|
||
| template <typename T> | ||
| RecordComponent &RecordComponent::makeConstant(T value) |
Check notice
Code scanning / CodeQL
No raw arrays in interfaces Note
|
Headers whose implementations could be moved to a .cpp file: include/openPMD/Span.hpp include/openPMD/Datatype.hpp // but keep constexpr implementations public some single functions: |
| namespace openPMD::auxiliary | ||
| { | ||
| std::unique_ptr<void, std::function<void(void *)>> | ||
| allocatePtr(Datatype dtype, uint64_t numPoints) |
Check warning
Code scanning / CodeQL
Poorly documented large function Warning
| // for (auto it = this->container().begin(); it != end; ++it) | ||
| // { | ||
| // if (it->first == RecordComponent::SCALAR) | ||
| // { | ||
| // this->container().erase(it); | ||
| // throw error::WrongAPIUsage(detail::NO_SCALAR_INSERT); | ||
| // } | ||
| // } |
Check notice
Code scanning / CodeQL
Commented-out code Note
| // for (auto it = this->container().begin(); it != end; ++it) | ||
| // { | ||
| // if (it->first == RecordComponent::SCALAR) | ||
| // { | ||
| // this->container().erase(it); | ||
| // throw error::WrongAPIUsage(detail::NO_SCALAR_INSERT); | ||
| // } | ||
| // } |
Check notice
Code scanning / CodeQL
Commented-out code Note
franzpoeschel
changed the title
ax3l
left a comment
ax3l
left a comment
There was a problem hiding this comment.
Awesome, thank you for digging this!
3 participants
Fix BLAST-WarpX/warpx#5984 (comment)
Including openPMD currently takes too much time. The main culprit seems to be our use of
std::variantfor all datatypes in the Datatype enum.This PR is somewhat macro-heavy to keep down compile times on our end.
std::variantin public headers ofopenPMD::Variant, move all use of it to object files, usestd::anypublically. This breaks the API, but I think that this part of the API was rarely used.It is somewhat likely that
std::variantwas very costly due to things likeAttribute::get<T>(): The requested typeTmust be matched against the actual typeA, leading to 38²=1444 instantiations. If naively pushing templates to the object files, this leads to very big compiled binaries. Some short-circuiting logic (pre-filling matrixes of which types are actually convertible and only instantiating in those cases when they are) helped in there.** Results **
The cut on compile times is significant. The SerialIOTests (which have gotten too huge and take ages to compile) used to compile in ~ a minute, now it's half a minute:
For a more normal example, I'll compile 10_streaming_write, which includes
openPMD.hppand does some basic transformations. Down to 2 seconds from 5 seconds:
Funny enough, the library size slightly shrinks from 9.8MB (Clang) / 8.5MB (g++) to 8.5MB (Clang) / 7.6MB (g++), despite pulling definitions into object files.
My guess is that most definitions were instantiated anyway, but the additional logic to cut away type conversion instantiations brings some size benefit.