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

Do not shrink serialization buffer upon EndStep – Fix Performance Bug #2516

Merged

Conversation

franzpoeschel
Copy link
Contributor

The BP4 serialization engine maintains the same serialization buffer format::BP4Serializer across steps. This allows to avoid heavyweight allocation operations after a certain "warmup" phase. Methods such as format::BPBase::ResizeBuffer support this idea by taking care not to shrink the buffer.

The method format::BP4Serializer::SerializeDataBuffer (called upon Engine::EndStep) does, however, pay no such attention.

This has noticeable consequences in writing to ADIOS2 via openPMD in PIConGPU. (Possibly related to the fact that the openPMD plugin in PIConGPU will repeatedly call Engine::PerformPuts, leading to a low buffer "fill status" upon Engine::EndStep, resulting in an actual shrinking of the buffer). While the buffer is never actually reallocated (which would happen only upon std::vector::shrink_to_fit), resizing it back up to the needed size in the following step involves zero-initialization. This means that ADIOS spends a lot of time per step just filling gigabytes of main memory with zeros. Quick measurements suggest a speedup down from ~1 minute for a data dump to ~30 seconds after fixing this.

This PR quickly fixes the issue by adding conditionals to avoid shrinking the buffer.

@pnorbert pnorbert merged commit 301b4e2 into ornladios:master Nov 19, 2020
@ax3l
Copy link
Contributor

ax3l commented Nov 30, 2020

Great find and fix, @franzpoeschel ! Thank you! 🚀 ✨

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.

4 participants