From e93a0b8d5822920a8a59f204591ec1cd585dcc48 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Tue, 17 Sep 2024 22:47:49 +0200 Subject: [PATCH 01/21] added new buffer class and unit tests --- src/Communicate/BufferHandler.h | 87 +++++++++++++++ unit_tests/Communicate/BufferHandler.cpp | 132 +++++++++++++++++++++++ unit_tests/Communicate/CMakeLists.txt | 23 ++++ 3 files changed, 242 insertions(+) create mode 100644 src/Communicate/BufferHandler.h create mode 100644 unit_tests/Communicate/BufferHandler.cpp create mode 100644 unit_tests/Communicate/CMakeLists.txt diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h new file mode 100644 index 000000000..1243d6d31 --- /dev/null +++ b/src/Communicate/BufferHandler.h @@ -0,0 +1,87 @@ +#ifndef IPPL_BUFFER_HANDLER_H +#define IPPL_BUFFER_HANDLER_H + +#include + +template +class BufferHandler { +public: + using archive_type = ippl::detail::Archive; + using buffer_type = std::shared_ptr; + using size_type = ippl::detail::size_type; + + ~BufferHandler() { + deleteAllBuffers(); + } + + buffer_type getBuffer(size_type size, double overallocation) { + size_type requiredSize = static_cast(size * overallocation); + + auto it = findSmallestSufficientBuffer(requiredSize); + + if (it != free_buffers.end()) { + buffer_type buffer = *it; + free_buffers.erase(it); + used_buffers.insert(buffer); + return buffer; + } + + if (!free_buffers.empty()) { + auto largest_it = std::prev(free_buffers.end()); + buffer_type buffer = *largest_it; + free_buffers.erase(largest_it); + + buffer->reallocBuffer(requiredSize); + + used_buffers.insert(buffer); + return buffer; + } + + buffer_type newBuffer = std::make_shared(requiredSize); + used_buffers.insert(newBuffer); + return newBuffer; + } + + void deleteBuffer(buffer_type buffer) { + auto it = used_buffers.find(buffer); + if (it != used_buffers.end()) { + used_buffers.erase(it); + free_buffers.insert(buffer); + } + } + + void freeAllBuffers() { + free_buffers.insert(used_buffers.begin(), used_buffers.end()); + used_buffers.clear(); + } + + void deleteAllBuffers() { + used_buffers.clear(); + free_buffers.clear(); + } + +protected: + struct BufferComparator { + bool operator()(const buffer_type& lhs, const buffer_type& rhs) const { + // First compare by size + if (lhs->getBufferSize() != rhs->getBufferSize()) { + return lhs->getBufferSize() < rhs->getBufferSize(); + } + // If sizes are equal, compare by address + return lhs < rhs; + } + }; + + typename std::set::iterator findSmallestSufficientBuffer( + size_type requiredSize) { + return std::find_if(free_buffers.begin(), free_buffers.end(), + [requiredSize](const buffer_type& buffer) { + return buffer->getBufferSize() >= requiredSize; + }); + } + + std::set used_buffers; + std::set free_buffers; +}; + +#endif diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp new file mode 100644 index 000000000..5458557e3 --- /dev/null +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -0,0 +1,132 @@ +#include "Ippl.h" + +#include "TestUtils.h" +#include "gtest/gtest.h" + +#include "Communicate/BufferHandler.h" + +using memory_space = typename Kokkos::View::memory_space; + +class TestableBufferHandler : public BufferHandler { +public: + using BufferHandler::deleteAllBuffers; + using BufferHandler::freeAllBuffers; + + size_t usedBuffersSize() const { return used_buffers.size(); } + + size_t freeBuffersSize() const { return free_buffers.size(); } +}; + +class BufferHandlerTest : public ::testing::Test { +protected: + void SetUp() override { handler = std::make_unique(); } + + void TearDown() override { handler.reset(); } + + std::unique_ptr handler; +}; + +TEST_F(BufferHandlerTest, GetBuffer_EmptyFreeBuffers) { + auto buffer = handler->getBuffer(100, 1.0); + ASSERT_NE(buffer, nullptr); + EXPECT_EQ(buffer->getBufferSize(), 100); + EXPECT_EQ(handler->usedBuffersSize(), 1); + EXPECT_EQ(handler->freeBuffersSize(), 0); +} + +TEST_F(BufferHandlerTest, GetBuffer_SuitableBufferAvailable) { + auto buffer1 = handler->getBuffer(50, 1.0); + handler->deleteBuffer(buffer1); + + auto buffer2 = handler->getBuffer(40, 1.0); + EXPECT_EQ(buffer2->getBufferSize(), 50); + EXPECT_EQ(handler->usedBuffersSize(), 1); + EXPECT_EQ(handler->freeBuffersSize(), 0); +} + +TEST_F(BufferHandlerTest, DeleteBuffer) { + auto buffer = handler->getBuffer(100, 1.0); + handler->deleteBuffer(buffer); + + EXPECT_EQ(handler->usedBuffersSize(), 0); + EXPECT_EQ(handler->freeBuffersSize(), 1); +} + +TEST_F(BufferHandlerTest, FreeAllBuffers) { + auto buffer1 = handler->getBuffer(50, 1.0); + auto buffer2 = handler->getBuffer(100, 1.0); + + handler->freeAllBuffers(); + + EXPECT_EQ(handler->usedBuffersSize(), 0); + EXPECT_EQ(handler->freeBuffersSize(), 2); +} + +TEST_F(BufferHandlerTest, DeleteAllBuffers) { + handler->getBuffer(50, 1.0); + handler->getBuffer(100, 1.0); + + handler->deleteAllBuffers(); + + EXPECT_EQ(handler->usedBuffersSize(), 0); + EXPECT_EQ(handler->freeBuffersSize(), 0); +} + +TEST_F(BufferHandlerTest, GetBuffer_ResizeLargerThanAvailable) { + auto smallBuffer = handler->getBuffer(50, 1.0); + handler->deleteBuffer(smallBuffer); + + auto largeBuffer = handler->getBuffer(200, 1.0); + EXPECT_EQ(largeBuffer->getBufferSize(), 200); + EXPECT_EQ(handler->usedBuffersSize(), 1); + EXPECT_EQ(handler->freeBuffersSize(), 0); +} + +TEST_F(BufferHandlerTest, GetBuffer_ExactSizeMatch) { + auto buffer1 = handler->getBuffer(100, 1.0); + handler->deleteBuffer(buffer1); + + auto buffer2 = handler->getBuffer(100, 1.0); + EXPECT_EQ(buffer2->getBufferSize(), 100); + EXPECT_EQ(handler->usedBuffersSize(), 1); + EXPECT_EQ(handler->freeBuffersSize(), 0); +} + +TEST_F(BufferHandlerTest, DeleteNonExistentBuffer) { + auto buffer = handler->getBuffer(100, 1.0); + auto newBuffer = std::make_shared>(200); + + handler->deleteBuffer(newBuffer); + EXPECT_EQ(handler->usedBuffersSize(), 1); + EXPECT_EQ(handler->freeBuffersSize(), 0); +} + +TEST_F(BufferHandlerTest, RepeatedAllocateAndFreeCycle) { + for (int i = 0; i < 10; ++i) { + auto buffer = handler->getBuffer(100, 1.0); + handler->deleteBuffer(buffer); + } + + EXPECT_EQ(handler->usedBuffersSize(), 0); + EXPECT_EQ(handler->freeBuffersSize(), 1); +} + +TEST_F(BufferHandlerTest, GetBuffer_ZeroSize) { + auto buffer = handler->getBuffer(0, 1.0); + ASSERT_NE(buffer, nullptr); + EXPECT_GE(buffer->getBufferSize(), 0); + EXPECT_EQ(handler->usedBuffersSize(), 1); + EXPECT_EQ(handler->freeBuffersSize(), 0); +} + + +int main(int argc, char* argv[]) { + int success = 1; + ippl::initialize(argc, argv); + { + ::testing::InitGoogleTest(&argc, argv); + success = RUN_ALL_TESTS(); + } + ippl::finalize(); + return success; +} diff --git a/unit_tests/Communicate/CMakeLists.txt b/unit_tests/Communicate/CMakeLists.txt new file mode 100644 index 000000000..369ec8574 --- /dev/null +++ b/unit_tests/Communicate/CMakeLists.txt @@ -0,0 +1,23 @@ +file (RELATIVE_PATH _relPath "${CMAKE_SOURCE_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}") +message (STATUS "Adding unit tests found in ${_relPath}") + +include_directories ( + ${CMAKE_SOURCE_DIR}/src + ${CMAKE_CURRENT_SOURCE_DIR}/.. +) + +link_directories ( + ${CMAKE_CURRENT_SOURCE_DIR} + ${Kokkos_DIR}/.. +) + +add_executable (BufferHandler BufferHandler.cpp) +gtest_discover_tests(BufferHandler PROPERTIES TEST_DISCOVERY_TIMEOUT 600) + +target_link_libraries ( + BufferHandler + ippl + pthread + GTest::gtest_main + ${MPI_CXX_LIBRARIES} +) From c7704c4fe03f64fac5a1ee59b6b79756f16793ef Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Sun, 22 Sep 2024 16:54:59 +0200 Subject: [PATCH 02/21] removed old buffer handler and integrated my buffer handler into ippl. The removal of the id has caused some unused parameter warnings. I also changed the structure of the BufferHandler a little bit to allow for a decorator pattern for the logger --- src/Communicate/BufferHandler.h | 78 ++++++++++++++++----- src/Communicate/Buffers.cpp | 11 ++- src/Communicate/Buffers.hpp | 27 +++---- src/Communicate/CMakeLists.txt | 1 + src/Communicate/Communicator.h | 21 +++--- src/Field/BcTypes.hpp | 7 +- src/Field/HaloCells.hpp | 7 +- src/Particle/ParticleBase.hpp | 4 +- src/PoissonSolvers/FFTOpenPoissonSolver.hpp | 15 ++-- unit_tests/Communicate/BufferHandler.cpp | 16 ++--- 10 files changed, 117 insertions(+), 70 deletions(-) diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h index 1243d6d31..a3ae83325 100644 --- a/src/Communicate/BufferHandler.h +++ b/src/Communicate/BufferHandler.h @@ -3,64 +3,103 @@ #include +#include "Communicate/Archive.h" + + template -class BufferHandler { +class IBufferHandler { public: using archive_type = ippl::detail::Archive; using buffer_type = std::shared_ptr; using size_type = ippl::detail::size_type; - ~BufferHandler() { - deleteAllBuffers(); - } + virtual ~IBufferHandler() {} + + virtual buffer_type getBuffer(size_type size, double overallocation) = 0; + virtual void freeBuffer(buffer_type buffer) = 0; + virtual void freeAllBuffers() = 0; + virtual void deleteAllBuffers() = 0; +protected: + virtual void insertBuffer(buffer_type buffer, bool isUsed) = 0; + virtual void eraseBuffer(buffer_type buffer, bool isUsed) = 0; +}; + +template +class BufferHandler : public IBufferHandler { +public: + using archive_type = ippl::detail::Archive; + using buffer_type = std::shared_ptr; + using size_type = ippl::detail::size_type; - buffer_type getBuffer(size_type size, double overallocation) { + ~BufferHandler() {} + + buffer_type getBuffer(size_type size, double overallocation) override { size_type requiredSize = static_cast(size * overallocation); auto it = findSmallestSufficientBuffer(requiredSize); if (it != free_buffers.end()) { buffer_type buffer = *it; - free_buffers.erase(it); - used_buffers.insert(buffer); + eraseBuffer(buffer, false); + insertBuffer(buffer, true); return buffer; } if (!free_buffers.empty()) { auto largest_it = std::prev(free_buffers.end()); buffer_type buffer = *largest_it; - free_buffers.erase(largest_it); - + eraseBuffer(buffer, false); buffer->reallocBuffer(requiredSize); - used_buffers.insert(buffer); + insertBuffer(buffer, true); return buffer; } buffer_type newBuffer = std::make_shared(requiredSize); - used_buffers.insert(newBuffer); + insertBuffer(newBuffer, true); return newBuffer; } - void deleteBuffer(buffer_type buffer) { + void freeBuffer(buffer_type buffer) override { auto it = used_buffers.find(buffer); if (it != used_buffers.end()) { - used_buffers.erase(it); - free_buffers.insert(buffer); + eraseBuffer(*it, true); + insertBuffer(*it, false); } } - void freeAllBuffers() { - free_buffers.insert(used_buffers.begin(), used_buffers.end()); - used_buffers.clear(); + void freeAllBuffers() override { + std::vector buffersToMove(used_buffers.begin(), used_buffers.end()); + + for (auto& buffer : buffersToMove) { + eraseBuffer(buffer, true); + insertBuffer(buffer, false); + } } - void deleteAllBuffers() { + void deleteAllBuffers() override { used_buffers.clear(); free_buffers.clear(); } - protected: + + void insertBuffer(buffer_type buffer, bool isUsed) override { + if (isUsed) { + used_buffers.insert(buffer); + } else { + free_buffers.insert(buffer); + } + } + + void eraseBuffer(buffer_type buffer, bool isUsed) override { + if (isUsed) { + used_buffers.erase(buffer); + } else { + free_buffers.erase(buffer); + } + } + +private: struct BufferComparator { bool operator()(const buffer_type& lhs, const buffer_type& rhs) const { // First compare by size @@ -80,6 +119,7 @@ class BufferHandler { }); } +protected: std::set used_buffers; std::set free_buffers; }; diff --git a/src/Communicate/Buffers.cpp b/src/Communicate/Buffers.cpp index cc0f0338e..04cbb1808 100644 --- a/src/Communicate/Buffers.cpp +++ b/src/Communicate/Buffers.cpp @@ -32,9 +32,16 @@ namespace ippl { } void Communicator::deleteAllBuffers() { - buffers_m.forAll([](Map&& m) { - m.clear(); + buffer_handlers_m.forAll([](BufferHandler&& bh) { + bh.deleteAllBuffers(); }); } + + void Communicator::freeAllBuffers() { + buffer_handlers_m.forAll([](BufferHandler&& bh) { + bh.freeAllBuffers(); + }); + } + } // namespace mpi } // namespace ippl diff --git a/src/Communicate/Buffers.hpp b/src/Communicate/Buffers.hpp index 94bbe4bd8..88a9982e9 100644 --- a/src/Communicate/Buffers.hpp +++ b/src/Communicate/Buffers.hpp @@ -24,20 +24,21 @@ namespace ippl { namespace mpi { template - Communicator::buffer_type Communicator::getBuffer(int id, size_type size, - double overallocation) { - auto& buffers = buffers_m.get(); - size *= sizeof(T); - if (buffers.contains(id)) { - if (buffers[id]->getBufferSize() < size) { - buffers[id]->reallocBuffer(size); - } - return buffers[id]; - } - buffers[id] = std::make_shared>( - (size_type)(size * std::max(overallocation, defaultOveralloc_m))); - return buffers[id]; + Communicator::buffer_type Communicator::getBufferr(size_type size, + double overallocation) { + auto& buffer_handler = buffer_handlers_m.get(); + + return buffer_handler.getBuffer(size * sizeof(T), std::max(overallocation, defaultOveralloc_m)); + } + + + template + void Communicator::freeBuffer(Communicator::buffer_type buffer) { + auto& buffer_handler = buffer_handlers_m.get(); + + return buffer_handler.freeBuffer(buffer); } + } // namespace mpi } // namespace ippl diff --git a/src/Communicate/CMakeLists.txt b/src/Communicate/CMakeLists.txt index 086f17710..9cf7e2240 100644 --- a/src/Communicate/CMakeLists.txt +++ b/src/Communicate/CMakeLists.txt @@ -6,6 +6,7 @@ set (_SRCS ) set (_HDRS + BufferHandler.h Archive.h Archive.hpp Buffers.hpp diff --git a/src/Communicate/Communicator.h b/src/Communicate/Communicator.h index 00280e9ce..2506ce227 100644 --- a/src/Communicate/Communicator.h +++ b/src/Communicate/Communicator.h @@ -8,6 +8,7 @@ #include #include +#include "Communicate/BufferHandler.h" #include "Communicate/Request.h" #include "Communicate/Status.h" @@ -132,10 +133,11 @@ namespace ippl { using buffer_type = std::shared_ptr>; private: + template - using map_type = std::map>; + using buffer_container_type = BufferHandler; - using buffer_map_type = typename detail::ContainerForAllSpaces::type; + using buffer_handler_type = typename detail::ContainerForAllSpaces::type; public: using size_type = detail::size_type; @@ -144,14 +146,14 @@ namespace ippl { template - buffer_type getBuffer(int id, size_type size, double overallocation = 1.0); - - template - void deleteBuffer(int id) { - buffers_m.get().erase(id); - } + buffer_type getBufferr(size_type size, double overallocation = 1.0); + void deleteAllBuffers(); + void freeAllBuffers(); + + template + void freeBuffer(buffer_type buffer); const MPI_Comm& getCommunicator() const noexcept { return *comm_m; } @@ -192,7 +194,8 @@ namespace ippl { } private: - buffer_map_type buffers_m; + buffer_handler_type buffer_handlers_m; + double defaultOveralloc_m = 1.0; ///////////////////////////////////////////////////////////////////////////////////// diff --git a/src/Field/BcTypes.hpp b/src/Field/BcTypes.hpp index fba6f65b5..e7538e993 100644 --- a/src/Field/BcTypes.hpp +++ b/src/Field/BcTypes.hpp @@ -262,8 +262,7 @@ namespace ippl { detail::size_type nSends; halo.pack(range, view, haloData_m, nSends); - buffer_type buf = comm.template getBuffer( - mpi::tag::PERIODIC_BC_SEND + i, nSends); + buffer_type buf = comm.template getBufferr(nSends); comm.isend(rank, tag, haloData_m, *buf, requests[i], nSends); buf->resetWritePos(); @@ -279,8 +278,7 @@ namespace ippl { detail::size_type nRecvs = range.size(); - buffer_type buf = comm.template getBuffer( - mpi::tag::PERIODIC_BC_RECV + i, nRecvs); + buffer_type buf = comm.template getBufferr(nRecvs); comm.recv(rank, matchtag, haloData_m, *buf, nRecvs * sizeof(T), nRecvs); buf->resetReadPos(); @@ -290,6 +288,7 @@ namespace ippl { if (!requests.empty()) { MPI_Waitall(requests.size(), requests.data(), MPI_STATUSES_IGNORE); } + comm.freeAllBuffers(); } // For all other processors do nothing } else { diff --git a/src/Field/HaloCells.hpp b/src/Field/HaloCells.hpp index 579e90f13..a019c49b2 100644 --- a/src/Field/HaloCells.hpp +++ b/src/Field/HaloCells.hpp @@ -71,8 +71,7 @@ namespace ippl { size_type nsends; pack(range, view, haloData_m, nsends); - buffer_type buf = comm.template getBuffer( - mpi::tag::HALO_SEND + i * cubeCount + index, nsends); + buffer_type buf = comm.template getBufferr(nsends); comm.isend(targetRank, tag, haloData_m, *buf, requests[requestIndex++], nsends); buf->resetWritePos(); @@ -95,8 +94,7 @@ namespace ippl { size_type nrecvs = range.size(); - buffer_type buf = comm.template getBuffer( - mpi::tag::HALO_RECV + i * cubeCount + index, nrecvs); + buffer_type buf = comm.template getBufferr(nrecvs); comm.recv(sourceRank, tag, haloData_m, *buf, nrecvs * sizeof(T), nrecvs); buf->resetReadPos(); @@ -108,6 +106,7 @@ namespace ippl { if (totalRequests > 0) { MPI_Waitall(totalRequests, requests.data(), MPI_STATUSES_IGNORE); } + comm.freeAllBuffers(); } template diff --git a/src/Particle/ParticleBase.hpp b/src/Particle/ParticleBase.hpp index 0ce60a61e..f908e5309 100644 --- a/src/Particle/ParticleBase.hpp +++ b/src/Particle/ParticleBase.hpp @@ -280,7 +280,7 @@ namespace ippl { return; } - auto buf = Comm->getBuffer(mpi::tag::PARTICLE_SEND + sendNum, bufSize); + auto buf = Comm->getBufferr(bufSize); Comm->isend(rank, tag++, *this, *buf, requests.back(), nSends); buf->resetWritePos(); @@ -296,7 +296,7 @@ namespace ippl { return; } - auto buf = Comm->getBuffer(mpi::tag::PARTICLE_RECV + recvNum, bufSize); + auto buf = Comm->getBufferr(bufSize); Comm->recv(rank, tag++, *this, *buf, bufSize, nRecvs); buf->resetReadPos(); diff --git a/src/PoissonSolvers/FFTOpenPoissonSolver.hpp b/src/PoissonSolvers/FFTOpenPoissonSolver.hpp index 74b25d002..1f53748c6 100644 --- a/src/PoissonSolvers/FFTOpenPoissonSolver.hpp +++ b/src/PoissonSolvers/FFTOpenPoissonSolver.hpp @@ -119,7 +119,7 @@ void solver_send(int BUF_MSG, int TAG, int id, int i, const ippl::NDIndex i // Buffer message indicates the domain intersection (x, y, z, xy, yz, xz, xyz). ippl::mpi::Communicator::buffer_type buf = - ippl::Comm->getBuffer(BUF_MSG + id * 8 + i, nsends); + ippl::Comm->getBufferr(nsends); int tag = TAG + id; @@ -140,7 +140,7 @@ void solver_recv(int BUF_MSG, int TAG, int id, int i, const ippl::NDIndex i // Buffer message indicates the domain intersection (x, y, z, xy, yz, xz, xyz). ippl::mpi::Communicator::buffer_type buf = - ippl::Comm->getBuffer(BUF_MSG + 8 * id + myRank, nrecvs); + ippl::Comm->getBufferr(nrecvs); int tag = TAG + id; @@ -571,7 +571,7 @@ namespace ippl { nrecvs = intersection.size(); buffer_type buf = - Comm->getBuffer(mpi::tag::SOLVER_RECV + myRank, nrecvs); + Comm->getBufferr(nrecvs); Comm->recv(i, mpi::tag::OPEN_SOLVER, fd_m, *buf, nrecvs * sizeof(Trhs), nrecvs); buf->resetReadPos(); @@ -696,8 +696,7 @@ namespace ippl { mpi::Communicator::size_type nrecvs; nrecvs = intersection.size(); - buffer_type buf = Comm->getBuffer( - mpi::tag::SOLVER_RECV + myRank, nrecvs); + buffer_type buf = Comm->getBufferr(nrecvs); Comm->recv(i, mpi::tag::OPEN_SOLVER, fd_m, *buf, nrecvs * sizeof(Trhs), nrecvs); @@ -853,8 +852,7 @@ namespace ippl { mpi::Communicator::size_type nrecvs; nrecvs = intersection.size(); - buffer_type buf = Comm->getBuffer( - mpi::tag::SOLVER_RECV + myRank, nrecvs); + buffer_type buf = Comm->getBufferr(nrecvs); Comm->recv(i, mpi::tag::OPEN_SOLVER, fd_m, *buf, nrecvs * sizeof(Trhs), nrecvs); @@ -1011,8 +1009,7 @@ namespace ippl { mpi::Communicator::size_type nrecvs; nrecvs = intersection.size(); - buffer_type buf = Comm->getBuffer( - mpi::tag::SOLVER_RECV + myRank, nrecvs); + buffer_type buf = Comm->getBufferr(nrecvs); Comm->recv(i, mpi::tag::OPEN_SOLVER, fd_m, *buf, nrecvs * sizeof(Trhs), nrecvs); diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp index 5458557e3..f75439d24 100644 --- a/unit_tests/Communicate/BufferHandler.cpp +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -36,7 +36,7 @@ TEST_F(BufferHandlerTest, GetBuffer_EmptyFreeBuffers) { TEST_F(BufferHandlerTest, GetBuffer_SuitableBufferAvailable) { auto buffer1 = handler->getBuffer(50, 1.0); - handler->deleteBuffer(buffer1); + handler->freeBuffer(buffer1); auto buffer2 = handler->getBuffer(40, 1.0); EXPECT_EQ(buffer2->getBufferSize(), 50); @@ -44,9 +44,9 @@ TEST_F(BufferHandlerTest, GetBuffer_SuitableBufferAvailable) { EXPECT_EQ(handler->freeBuffersSize(), 0); } -TEST_F(BufferHandlerTest, DeleteBuffer) { +TEST_F(BufferHandlerTest, FreeBuffer) { auto buffer = handler->getBuffer(100, 1.0); - handler->deleteBuffer(buffer); + handler->freeBuffer(buffer); EXPECT_EQ(handler->usedBuffersSize(), 0); EXPECT_EQ(handler->freeBuffersSize(), 1); @@ -74,7 +74,7 @@ TEST_F(BufferHandlerTest, DeleteAllBuffers) { TEST_F(BufferHandlerTest, GetBuffer_ResizeLargerThanAvailable) { auto smallBuffer = handler->getBuffer(50, 1.0); - handler->deleteBuffer(smallBuffer); + handler->freeBuffer(smallBuffer); auto largeBuffer = handler->getBuffer(200, 1.0); EXPECT_EQ(largeBuffer->getBufferSize(), 200); @@ -84,7 +84,7 @@ TEST_F(BufferHandlerTest, GetBuffer_ResizeLargerThanAvailable) { TEST_F(BufferHandlerTest, GetBuffer_ExactSizeMatch) { auto buffer1 = handler->getBuffer(100, 1.0); - handler->deleteBuffer(buffer1); + handler->freeBuffer(buffer1); auto buffer2 = handler->getBuffer(100, 1.0); EXPECT_EQ(buffer2->getBufferSize(), 100); @@ -92,11 +92,11 @@ TEST_F(BufferHandlerTest, GetBuffer_ExactSizeMatch) { EXPECT_EQ(handler->freeBuffersSize(), 0); } -TEST_F(BufferHandlerTest, DeleteNonExistentBuffer) { +TEST_F(BufferHandlerTest, FreeNonExistentBuffer) { auto buffer = handler->getBuffer(100, 1.0); auto newBuffer = std::make_shared>(200); - handler->deleteBuffer(newBuffer); + handler->freeBuffer(newBuffer); EXPECT_EQ(handler->usedBuffersSize(), 1); EXPECT_EQ(handler->freeBuffersSize(), 0); } @@ -104,7 +104,7 @@ TEST_F(BufferHandlerTest, DeleteNonExistentBuffer) { TEST_F(BufferHandlerTest, RepeatedAllocateAndFreeCycle) { for (int i = 0; i < 10; ++i) { auto buffer = handler->getBuffer(100, 1.0); - handler->deleteBuffer(buffer); + handler->freeBuffer(buffer); } EXPECT_EQ(handler->usedBuffersSize(), 0); From 7a37e857c72377e9c062dc6a96b6d3b69945f8f2 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Sun, 22 Sep 2024 17:10:08 +0200 Subject: [PATCH 03/21] changed debug-name for get buffer method --- src/Communicate/Buffers.hpp | 2 +- src/Communicate/Communicator.h | 2 +- src/Field/BcTypes.hpp | 4 ++-- src/Field/HaloCells.hpp | 4 ++-- src/Particle/ParticleBase.hpp | 4 ++-- src/PoissonSolvers/FFTOpenPoissonSolver.hpp | 12 ++++++------ 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Communicate/Buffers.hpp b/src/Communicate/Buffers.hpp index 88a9982e9..7fd72860d 100644 --- a/src/Communicate/Buffers.hpp +++ b/src/Communicate/Buffers.hpp @@ -24,7 +24,7 @@ namespace ippl { namespace mpi { template - Communicator::buffer_type Communicator::getBufferr(size_type size, + Communicator::buffer_type Communicator::getBuffer(size_type size, double overallocation) { auto& buffer_handler = buffer_handlers_m.get(); diff --git a/src/Communicate/Communicator.h b/src/Communicate/Communicator.h index 2506ce227..a7df873f9 100644 --- a/src/Communicate/Communicator.h +++ b/src/Communicate/Communicator.h @@ -146,7 +146,7 @@ namespace ippl { template - buffer_type getBufferr(size_type size, double overallocation = 1.0); + buffer_type getBuffer(size_type size, double overallocation = 1.0); void deleteAllBuffers(); diff --git a/src/Field/BcTypes.hpp b/src/Field/BcTypes.hpp index e7538e993..4cc10e720 100644 --- a/src/Field/BcTypes.hpp +++ b/src/Field/BcTypes.hpp @@ -262,7 +262,7 @@ namespace ippl { detail::size_type nSends; halo.pack(range, view, haloData_m, nSends); - buffer_type buf = comm.template getBufferr(nSends); + buffer_type buf = comm.template getBuffer(nSends); comm.isend(rank, tag, haloData_m, *buf, requests[i], nSends); buf->resetWritePos(); @@ -278,7 +278,7 @@ namespace ippl { detail::size_type nRecvs = range.size(); - buffer_type buf = comm.template getBufferr(nRecvs); + buffer_type buf = comm.template getBuffer(nRecvs); comm.recv(rank, matchtag, haloData_m, *buf, nRecvs * sizeof(T), nRecvs); buf->resetReadPos(); diff --git a/src/Field/HaloCells.hpp b/src/Field/HaloCells.hpp index a019c49b2..34841f1ae 100644 --- a/src/Field/HaloCells.hpp +++ b/src/Field/HaloCells.hpp @@ -71,7 +71,7 @@ namespace ippl { size_type nsends; pack(range, view, haloData_m, nsends); - buffer_type buf = comm.template getBufferr(nsends); + buffer_type buf = comm.template getBuffer(nsends); comm.isend(targetRank, tag, haloData_m, *buf, requests[requestIndex++], nsends); buf->resetWritePos(); @@ -94,7 +94,7 @@ namespace ippl { size_type nrecvs = range.size(); - buffer_type buf = comm.template getBufferr(nrecvs); + buffer_type buf = comm.template getBuffer(nrecvs); comm.recv(sourceRank, tag, haloData_m, *buf, nrecvs * sizeof(T), nrecvs); buf->resetReadPos(); diff --git a/src/Particle/ParticleBase.hpp b/src/Particle/ParticleBase.hpp index f908e5309..dc937b3bb 100644 --- a/src/Particle/ParticleBase.hpp +++ b/src/Particle/ParticleBase.hpp @@ -280,7 +280,7 @@ namespace ippl { return; } - auto buf = Comm->getBufferr(bufSize); + auto buf = Comm->getBuffer(bufSize); Comm->isend(rank, tag++, *this, *buf, requests.back(), nSends); buf->resetWritePos(); @@ -296,7 +296,7 @@ namespace ippl { return; } - auto buf = Comm->getBufferr(bufSize); + auto buf = Comm->getBuffer(bufSize); Comm->recv(rank, tag++, *this, *buf, bufSize, nRecvs); buf->resetReadPos(); diff --git a/src/PoissonSolvers/FFTOpenPoissonSolver.hpp b/src/PoissonSolvers/FFTOpenPoissonSolver.hpp index 1f53748c6..b842449a7 100644 --- a/src/PoissonSolvers/FFTOpenPoissonSolver.hpp +++ b/src/PoissonSolvers/FFTOpenPoissonSolver.hpp @@ -119,7 +119,7 @@ void solver_send(int BUF_MSG, int TAG, int id, int i, const ippl::NDIndex i // Buffer message indicates the domain intersection (x, y, z, xy, yz, xz, xyz). ippl::mpi::Communicator::buffer_type buf = - ippl::Comm->getBufferr(nsends); + ippl::Comm->getBuffer(nsends); int tag = TAG + id; @@ -140,7 +140,7 @@ void solver_recv(int BUF_MSG, int TAG, int id, int i, const ippl::NDIndex i // Buffer message indicates the domain intersection (x, y, z, xy, yz, xz, xyz). ippl::mpi::Communicator::buffer_type buf = - ippl::Comm->getBufferr(nrecvs); + ippl::Comm->getBuffer(nrecvs); int tag = TAG + id; @@ -571,7 +571,7 @@ namespace ippl { nrecvs = intersection.size(); buffer_type buf = - Comm->getBufferr(nrecvs); + Comm->getBuffer(nrecvs); Comm->recv(i, mpi::tag::OPEN_SOLVER, fd_m, *buf, nrecvs * sizeof(Trhs), nrecvs); buf->resetReadPos(); @@ -696,7 +696,7 @@ namespace ippl { mpi::Communicator::size_type nrecvs; nrecvs = intersection.size(); - buffer_type buf = Comm->getBufferr(nrecvs); + buffer_type buf = Comm->getBuffer(nrecvs); Comm->recv(i, mpi::tag::OPEN_SOLVER, fd_m, *buf, nrecvs * sizeof(Trhs), nrecvs); @@ -852,7 +852,7 @@ namespace ippl { mpi::Communicator::size_type nrecvs; nrecvs = intersection.size(); - buffer_type buf = Comm->getBufferr(nrecvs); + buffer_type buf = Comm->getBuffer(nrecvs); Comm->recv(i, mpi::tag::OPEN_SOLVER, fd_m, *buf, nrecvs * sizeof(Trhs), nrecvs); @@ -1009,7 +1009,7 @@ namespace ippl { mpi::Communicator::size_type nrecvs; nrecvs = intersection.size(); - buffer_type buf = Comm->getBufferr(nrecvs); + buffer_type buf = Comm->getBuffer(nrecvs); Comm->recv(i, mpi::tag::OPEN_SOLVER, fd_m, *buf, nrecvs * sizeof(Trhs), nrecvs); From 797f9003ac95d919c9a08a7ea249184cd5825443 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Sun, 29 Sep 2024 20:13:43 +0200 Subject: [PATCH 04/21] changed interface to allow for logging wrapper class with unit tests --- src/Communicate/BufferHandler.h | 77 ++++++++++++++---------- unit_tests/Communicate/BufferHandler.cpp | 59 ++++++++++++++++++ 2 files changed, 104 insertions(+), 32 deletions(-) diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h index a3ae83325..3b16edad4 100644 --- a/src/Communicate/BufferHandler.h +++ b/src/Communicate/BufferHandler.h @@ -20,8 +20,8 @@ class IBufferHandler { virtual void freeAllBuffers() = 0; virtual void deleteAllBuffers() = 0; protected: - virtual void insertBuffer(buffer_type buffer, bool isUsed) = 0; - virtual void eraseBuffer(buffer_type buffer, bool isUsed) = 0; + virtual size_type getAllocatedSize() const = 0; + virtual size_type getFreeSize() const = 0; }; template @@ -40,63 +40,63 @@ class BufferHandler : public IBufferHandler { if (it != free_buffers.end()) { buffer_type buffer = *it; - eraseBuffer(buffer, false); - insertBuffer(buffer, true); + + freeSize -= buffer->getBufferSize(); + allocatedSize += buffer->getBufferSize(); + + free_buffers.erase(buffer); + used_buffers.insert(buffer); return buffer; } if (!free_buffers.empty()) { auto largest_it = std::prev(free_buffers.end()); buffer_type buffer = *largest_it; - eraseBuffer(buffer, false); + + freeSize -= buffer->getBufferSize(); + allocatedSize += requiredSize; + + free_buffers.erase(buffer); buffer->reallocBuffer(requiredSize); - insertBuffer(buffer, true); + used_buffers.insert(buffer); return buffer; } buffer_type newBuffer = std::make_shared(requiredSize); - insertBuffer(newBuffer, true); + + allocatedSize += newBuffer->getBufferSize(); + + used_buffers.insert(newBuffer); return newBuffer; } void freeBuffer(buffer_type buffer) override { auto it = used_buffers.find(buffer); if (it != used_buffers.end()) { - eraseBuffer(*it, true); - insertBuffer(*it, false); - } - } - void freeAllBuffers() override { - std::vector buffersToMove(used_buffers.begin(), used_buffers.end()); + allocatedSize -= (*it)->getBufferSize(); + freeSize += (*it)->getBufferSize(); - for (auto& buffer : buffersToMove) { - eraseBuffer(buffer, true); - insertBuffer(buffer, false); + used_buffers.erase(*it); + free_buffers.insert(*it); } } - void deleteAllBuffers() override { + void freeAllBuffers() override { + freeSize += allocatedSize; + allocatedSize = 0; + + free_buffers.insert(used_buffers.begin(), used_buffers.end()); used_buffers.clear(); - free_buffers.clear(); } -protected: - void insertBuffer(buffer_type buffer, bool isUsed) override { - if (isUsed) { - used_buffers.insert(buffer); - } else { - free_buffers.insert(buffer); - } - } + void deleteAllBuffers() override { + freeSize = 0; + allocatedSize = 0; - void eraseBuffer(buffer_type buffer, bool isUsed) override { - if (isUsed) { - used_buffers.erase(buffer); - } else { - free_buffers.erase(buffer); - } + used_buffers.clear(); + free_buffers.clear(); } private: @@ -118,10 +118,23 @@ class BufferHandler : public IBufferHandler { return buffer->getBufferSize() >= requiredSize; }); } + + size_type allocatedSize; + size_type freeSize; + protected: std::set used_buffers; std::set free_buffers; + + size_type getAllocatedSize() const override { + return allocatedSize; + } + + size_type getFreeSize() const override { + return freeSize; + } + }; #endif diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp index f75439d24..4a9a24541 100644 --- a/unit_tests/Communicate/BufferHandler.cpp +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -15,6 +15,14 @@ class TestableBufferHandler : public BufferHandler { size_t usedBuffersSize() const { return used_buffers.size(); } size_t freeBuffersSize() const { return free_buffers.size(); } + + size_t getAllocatedSize() const { + return BufferHandler::getAllocatedSize(); + } + + size_t getFreeSize() const { + return BufferHandler::getFreeSize(); + } }; class BufferHandlerTest : public ::testing::Test { @@ -119,6 +127,57 @@ TEST_F(BufferHandlerTest, GetBuffer_ZeroSize) { EXPECT_EQ(handler->freeBuffersSize(), 0); } +TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_EmptyHandler) { + EXPECT_EQ(handler->getAllocatedSize(), 0); + EXPECT_EQ(handler->getFreeSize(), 0); +} + +TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_AfterBufferAllocation) { + auto buffer = handler->getBuffer(100, 1.0); + EXPECT_EQ(handler->getAllocatedSize(), 100); + EXPECT_EQ(handler->getFreeSize(), 0); +} + +TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeBuffer) { + auto buffer = handler->getBuffer(100, 1.0); + handler->freeBuffer(buffer); + + EXPECT_EQ(handler->getAllocatedSize(), 0); + EXPECT_EQ(handler->getFreeSize(), 100); +} + +TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeAllBuffers) { + auto buffer1 = handler->getBuffer(50, 1.0); + auto buffer2 = handler->getBuffer(100, 1.0); + + handler->freeAllBuffers(); + + EXPECT_EQ(handler->getAllocatedSize(), 0); + EXPECT_EQ(handler->getFreeSize(), 150); +} + +TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_AfterDeleteAllBuffers) { + handler->getBuffer(50, 1.0); + handler->getBuffer(100, 1.0); + + handler->deleteAllBuffers(); + + EXPECT_EQ(handler->getAllocatedSize(), 0); + EXPECT_EQ(handler->getFreeSize(), 0); +} + +TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_ResizeBufferLargerThanAvailable) { + auto smallBuffer = handler->getBuffer(50, 1.0); + handler->freeBuffer(smallBuffer); + + auto largeBuffer = handler->getBuffer(200, 1.0); + + EXPECT_EQ(handler->getAllocatedSize(), 200); + EXPECT_EQ(handler->getFreeSize(), 0); +} + + + int main(int argc, char* argv[]) { int success = 1; From 18b8fcac664a73c89e6738a369c8ccb0b02af74e Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Thu, 3 Oct 2024 14:06:54 +0200 Subject: [PATCH 05/21] made methods for size public --- src/Communicate/BufferHandler.h | 18 +++++++++--------- unit_tests/Communicate/BufferHandler.cpp | 7 ------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h index 3b16edad4..51a57de08 100644 --- a/src/Communicate/BufferHandler.h +++ b/src/Communicate/BufferHandler.h @@ -19,7 +19,7 @@ class IBufferHandler { virtual void freeBuffer(buffer_type buffer) = 0; virtual void freeAllBuffers() = 0; virtual void deleteAllBuffers() = 0; -protected: + virtual size_type getAllocatedSize() const = 0; virtual size_type getFreeSize() const = 0; }; @@ -99,6 +99,14 @@ class BufferHandler : public IBufferHandler { free_buffers.clear(); } + size_type getAllocatedSize() const override { + return allocatedSize; + } + + size_type getFreeSize() const override { + return freeSize; + } + private: struct BufferComparator { bool operator()(const buffer_type& lhs, const buffer_type& rhs) const { @@ -127,14 +135,6 @@ class BufferHandler : public IBufferHandler { std::set used_buffers; std::set free_buffers; - size_type getAllocatedSize() const override { - return allocatedSize; - } - - size_type getFreeSize() const override { - return freeSize; - } - }; #endif diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp index 4a9a24541..1faae466f 100644 --- a/unit_tests/Communicate/BufferHandler.cpp +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -16,13 +16,6 @@ class TestableBufferHandler : public BufferHandler { size_t freeBuffersSize() const { return free_buffers.size(); } - size_t getAllocatedSize() const { - return BufferHandler::getAllocatedSize(); - } - - size_t getFreeSize() const { - return BufferHandler::getFreeSize(); - } }; class BufferHandlerTest : public ::testing::Test { From 3aa99f991e2430de7f5db03278d7109a5b11b551 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Thu, 3 Oct 2024 14:39:59 +0200 Subject: [PATCH 06/21] made bufferhandler more readable --- src/Communicate/BufferHandler.h | 137 +++++++++++++++++++------------- 1 file changed, 81 insertions(+), 56 deletions(-) diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h index 51a57de08..dbeb8ff93 100644 --- a/src/Communicate/BufferHandler.h +++ b/src/Communicate/BufferHandler.h @@ -36,59 +36,31 @@ class BufferHandler : public IBufferHandler { buffer_type getBuffer(size_type size, double overallocation) override { size_type requiredSize = static_cast(size * overallocation); - auto it = findSmallestSufficientBuffer(requiredSize); - - if (it != free_buffers.end()) { - buffer_type buffer = *it; - - freeSize -= buffer->getBufferSize(); - allocatedSize += buffer->getBufferSize(); + auto freeBuffer = findFreeBuffer(requiredSize); - free_buffers.erase(buffer); - used_buffers.insert(buffer); - return buffer; + if (freeBuffer != nullptr) { + return allocateFromFreeBuffer(freeBuffer); } if (!free_buffers.empty()) { - auto largest_it = std::prev(free_buffers.end()); - buffer_type buffer = *largest_it; - - freeSize -= buffer->getBufferSize(); - allocatedSize += requiredSize; - - free_buffers.erase(buffer); - buffer->reallocBuffer(requiredSize); - - used_buffers.insert(buffer); - return buffer; + return reallocateLargestFreeBuffer(requiredSize); } - buffer_type newBuffer = std::make_shared(requiredSize); - - allocatedSize += newBuffer->getBufferSize(); - - used_buffers.insert(newBuffer); - return newBuffer; + return allocateNewBuffer(requiredSize); } void freeBuffer(buffer_type buffer) override { - auto it = used_buffers.find(buffer); - if (it != used_buffers.end()) { - - allocatedSize -= (*it)->getBufferSize(); - freeSize += (*it)->getBufferSize(); - - used_buffers.erase(*it); - free_buffers.insert(*it); + if (isBufferUsed(buffer)) { + releaseUsedBuffer(buffer); } } void freeAllBuffers() override { - freeSize += allocatedSize; - allocatedSize = 0; - free_buffers.insert(used_buffers.begin(), used_buffers.end()); used_buffers.clear(); + + freeSize += allocatedSize; + allocatedSize = 0; } void deleteAllBuffers() override { @@ -99,41 +71,94 @@ class BufferHandler : public IBufferHandler { free_buffers.clear(); } - size_type getAllocatedSize() const override { - return allocatedSize; - } + size_type getAllocatedSize() const override { + return allocatedSize; + } - size_type getFreeSize() const override { - return freeSize; - } + size_type getFreeSize() const override { + return freeSize; + } private: - struct BufferComparator { - bool operator()(const buffer_type& lhs, const buffer_type& rhs) const { - // First compare by size - if (lhs->getBufferSize() != rhs->getBufferSize()) { - return lhs->getBufferSize() < rhs->getBufferSize(); - } - // If sizes are equal, compare by address - return lhs < rhs; + using buffer_comparator_type = bool (*)(const buffer_type&, const buffer_type&); + + static bool bufferSizeComparator(const buffer_type& lhs, const buffer_type& rhs) { + // Compare by size first, then by memory address + if (lhs->getBufferSize() != rhs->getBufferSize()) { + return lhs->getBufferSize() < rhs->getBufferSize(); + } + return lhs < rhs; + } + + bool isBufferUsed(buffer_type buffer) const { + return used_buffers.find(buffer) != used_buffers.end(); + } + + void releaseUsedBuffer(buffer_type buffer) { + auto it = used_buffers.find(buffer); + + allocatedSize -= buffer->getBufferSize(); + freeSize += buffer->getBufferSize(); + + used_buffers.erase(it); + free_buffers.insert(buffer); + } + + buffer_type findFreeBuffer(size_type requiredSize) { + auto it = findSmallestSufficientBuffer(requiredSize); + if (it != free_buffers.end()) { + return *it; } - }; + return nullptr; + } - typename std::set::iterator findSmallestSufficientBuffer( + typename std::set::iterator findSmallestSufficientBuffer( size_type requiredSize) { return std::find_if(free_buffers.begin(), free_buffers.end(), [requiredSize](const buffer_type& buffer) { return buffer->getBufferSize() >= requiredSize; }); } + + + buffer_type allocateFromFreeBuffer(buffer_type buffer) { + freeSize -= buffer->getBufferSize(); + allocatedSize += buffer->getBufferSize(); + + free_buffers.erase(buffer); + used_buffers.insert(buffer); + return buffer; + } + + buffer_type reallocateLargestFreeBuffer(size_type requiredSize) { + auto largest_it = std::prev(free_buffers.end()); + buffer_type buffer = *largest_it; + + freeSize -= buffer->getBufferSize(); + allocatedSize += requiredSize; + + free_buffers.erase(buffer); + buffer->reallocBuffer(requiredSize); + + used_buffers.insert(buffer); + return buffer; + } + + buffer_type allocateNewBuffer(size_type requiredSize) { + buffer_type newBuffer = std::make_shared(requiredSize); + + allocatedSize += newBuffer->getBufferSize(); + used_buffers.insert(newBuffer); + return newBuffer; + } size_type allocatedSize; size_type freeSize; protected: - std::set used_buffers; - std::set free_buffers; + std::set used_buffers{&BufferHandler::bufferSizeComparator}; + std::set free_buffers{&BufferHandler::bufferSizeComparator}; }; From af3576192e69757e662eee086645b1cb5b1bca73 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Thu, 3 Oct 2024 15:22:17 +0200 Subject: [PATCH 07/21] run tests for every memory space --- src/Communicate/BufferHandler.h | 40 ++--- unit_tests/Communicate/BufferHandler.cpp | 213 ++++++++++++----------- 2 files changed, 129 insertions(+), 124 deletions(-) diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h index dbeb8ff93..33b4f96a9 100644 --- a/src/Communicate/BufferHandler.h +++ b/src/Communicate/BufferHandler.h @@ -5,7 +5,6 @@ #include "Communicate/Archive.h" - template class IBufferHandler { public: @@ -16,12 +15,12 @@ class IBufferHandler { virtual ~IBufferHandler() {} virtual buffer_type getBuffer(size_type size, double overallocation) = 0; - virtual void freeBuffer(buffer_type buffer) = 0; - virtual void freeAllBuffers() = 0; - virtual void deleteAllBuffers() = 0; + virtual void freeBuffer(buffer_type buffer) = 0; + virtual void freeAllBuffers() = 0; + virtual void deleteAllBuffers() = 0; virtual size_type getAllocatedSize() const = 0; - virtual size_type getFreeSize() const = 0; + virtual size_type getFreeSize() const = 0; }; template @@ -64,26 +63,23 @@ class BufferHandler : public IBufferHandler { } void deleteAllBuffers() override { - freeSize = 0; + freeSize = 0; allocatedSize = 0; used_buffers.clear(); free_buffers.clear(); } - size_type getAllocatedSize() const override { - return allocatedSize; - } + size_type getAllocatedSize() const override { return allocatedSize; } - size_type getFreeSize() const override { - return freeSize; - } + size_type getFreeSize() const override { return freeSize; } private: using buffer_comparator_type = bool (*)(const buffer_type&, const buffer_type&); + using buffer_set_type = std::set; static bool bufferSizeComparator(const buffer_type& lhs, const buffer_type& rhs) { - // Compare by size first, then by memory address + // Compare by size first, then by memory address to get total ordering if (lhs->getBufferSize() != rhs->getBufferSize()) { return lhs->getBufferSize() < rhs->getBufferSize(); } @@ -96,7 +92,7 @@ class BufferHandler : public IBufferHandler { void releaseUsedBuffer(buffer_type buffer) { auto it = used_buffers.find(buffer); - + allocatedSize -= buffer->getBufferSize(); freeSize += buffer->getBufferSize(); @@ -112,15 +108,13 @@ class BufferHandler : public IBufferHandler { return nullptr; } - typename std::set::iterator findSmallestSufficientBuffer( - size_type requiredSize) { + buffer_set_type::iterator findSmallestSufficientBuffer(size_type requiredSize) { return std::find_if(free_buffers.begin(), free_buffers.end(), [requiredSize](const buffer_type& buffer) { return buffer->getBufferSize() >= requiredSize; }); } - buffer_type allocateFromFreeBuffer(buffer_type buffer) { freeSize -= buffer->getBufferSize(); allocatedSize += buffer->getBufferSize(); @@ -131,7 +125,7 @@ class BufferHandler : public IBufferHandler { } buffer_type reallocateLargestFreeBuffer(size_type requiredSize) { - auto largest_it = std::prev(free_buffers.end()); + auto largest_it = std::prev(free_buffers.end()); buffer_type buffer = *largest_it; freeSize -= buffer->getBufferSize(); @@ -151,15 +145,13 @@ class BufferHandler : public IBufferHandler { used_buffers.insert(newBuffer); return newBuffer; } - - size_type allocatedSize; - size_type freeSize; + size_type allocatedSize; + size_type freeSize; protected: - std::set used_buffers{&BufferHandler::bufferSizeComparator}; - std::set free_buffers{&BufferHandler::bufferSizeComparator}; - + buffer_set_type used_buffers{&BufferHandler::bufferSizeComparator}; + buffer_set_type free_buffers{&BufferHandler::bufferSizeComparator}; }; #endif diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp index 1faae466f..1843b877c 100644 --- a/unit_tests/Communicate/BufferHandler.cpp +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -1,25 +1,40 @@ #include "Ippl.h" +#include "Communicate/BufferHandler.h" + #include "TestUtils.h" #include "gtest/gtest.h" -#include "Communicate/BufferHandler.h" -using memory_space = typename Kokkos::View::memory_space; +//using MemorySpaces = typename ippl::detail::TypeForAllSpaces::unique_memory_spaces; -class TestableBufferHandler : public BufferHandler { -public: - using BufferHandler::deleteAllBuffers; - using BufferHandler::freeAllBuffers; - size_t usedBuffersSize() const { return used_buffers.size(); } - - size_t freeBuffersSize() const { return free_buffers.size(); } +template +struct VariantToTypes; +template +struct VariantToTypes> { + using type = ::testing::Types; }; -class BufferHandlerTest : public ::testing::Test { +using MemorySpaces = typename VariantToTypes::unique_memory_spaces>::type; + + +template +class TypedBufferHandlerTest : public ::testing::Test { protected: + using memory_space = MemorySpace; + + class TestableBufferHandler : public BufferHandler { + public: + using BufferHandler::deleteAllBuffers; + using BufferHandler::freeAllBuffers; + + size_t usedBuffersSize() const { return this->used_buffers.size(); } + + size_t freeBuffersSize() const { return this->free_buffers.size(); } + }; + void SetUp() override { handler = std::make_unique(); } void TearDown() override { handler.reset(); } @@ -27,150 +42,148 @@ class BufferHandlerTest : public ::testing::Test { std::unique_ptr handler; }; -TEST_F(BufferHandlerTest, GetBuffer_EmptyFreeBuffers) { - auto buffer = handler->getBuffer(100, 1.0); +TYPED_TEST_CASE(TypedBufferHandlerTest, MemorySpaces); + +TYPED_TEST(TypedBufferHandlerTest, GetBuffer_EmptyFreeBuffers) { + auto buffer = this->handler->getBuffer(100, 1.0); ASSERT_NE(buffer, nullptr); EXPECT_EQ(buffer->getBufferSize(), 100); - EXPECT_EQ(handler->usedBuffersSize(), 1); - EXPECT_EQ(handler->freeBuffersSize(), 0); + EXPECT_EQ(this->handler->usedBuffersSize(), 1); + EXPECT_EQ(this->handler->freeBuffersSize(), 0); } -TEST_F(BufferHandlerTest, GetBuffer_SuitableBufferAvailable) { - auto buffer1 = handler->getBuffer(50, 1.0); - handler->freeBuffer(buffer1); +TYPED_TEST(TypedBufferHandlerTest, GetBuffer_SuitableBufferAvailable) { + auto buffer1 = this->handler->getBuffer(50, 1.0); + this->handler->freeBuffer(buffer1); - auto buffer2 = handler->getBuffer(40, 1.0); + auto buffer2 = this->handler->getBuffer(40, 1.0); EXPECT_EQ(buffer2->getBufferSize(), 50); - EXPECT_EQ(handler->usedBuffersSize(), 1); - EXPECT_EQ(handler->freeBuffersSize(), 0); + EXPECT_EQ(this->handler->usedBuffersSize(), 1); + EXPECT_EQ(this->handler->freeBuffersSize(), 0); } +TYPED_TEST(TypedBufferHandlerTest, FreeBuffer) { + auto buffer = this->handler->getBuffer(100, 1.0); + this->handler->freeBuffer(buffer); -TEST_F(BufferHandlerTest, FreeBuffer) { - auto buffer = handler->getBuffer(100, 1.0); - handler->freeBuffer(buffer); - - EXPECT_EQ(handler->usedBuffersSize(), 0); - EXPECT_EQ(handler->freeBuffersSize(), 1); + EXPECT_EQ(this->handler->usedBuffersSize(), 0); + EXPECT_EQ(this->handler->freeBuffersSize(), 1); } -TEST_F(BufferHandlerTest, FreeAllBuffers) { - auto buffer1 = handler->getBuffer(50, 1.0); - auto buffer2 = handler->getBuffer(100, 1.0); +TYPED_TEST(TypedBufferHandlerTest, FreeAllBuffers) { + auto buffer1 = this->handler->getBuffer(50, 1.0); + auto buffer2 = this->handler->getBuffer(100, 1.0); - handler->freeAllBuffers(); + this->handler->freeAllBuffers(); - EXPECT_EQ(handler->usedBuffersSize(), 0); - EXPECT_EQ(handler->freeBuffersSize(), 2); + EXPECT_EQ(this->handler->usedBuffersSize(), 0); + EXPECT_EQ(this->handler->freeBuffersSize(), 2); } -TEST_F(BufferHandlerTest, DeleteAllBuffers) { - handler->getBuffer(50, 1.0); - handler->getBuffer(100, 1.0); +TYPED_TEST(TypedBufferHandlerTest, DeleteAllBuffers) { + this->handler->getBuffer(50, 1.0); + this->handler->getBuffer(100, 1.0); - handler->deleteAllBuffers(); + this->handler->deleteAllBuffers(); - EXPECT_EQ(handler->usedBuffersSize(), 0); - EXPECT_EQ(handler->freeBuffersSize(), 0); + EXPECT_EQ(this->handler->usedBuffersSize(), 0); + EXPECT_EQ(this->handler->freeBuffersSize(), 0); } -TEST_F(BufferHandlerTest, GetBuffer_ResizeLargerThanAvailable) { - auto smallBuffer = handler->getBuffer(50, 1.0); - handler->freeBuffer(smallBuffer); +TYPED_TEST(TypedBufferHandlerTest, GetBuffer_ResizeLargerThanAvailable) { + auto smallBuffer = this->handler->getBuffer(50, 1.0); + this->handler->freeBuffer(smallBuffer); - auto largeBuffer = handler->getBuffer(200, 1.0); + auto largeBuffer = this->handler->getBuffer(200, 1.0); EXPECT_EQ(largeBuffer->getBufferSize(), 200); - EXPECT_EQ(handler->usedBuffersSize(), 1); - EXPECT_EQ(handler->freeBuffersSize(), 0); + EXPECT_EQ(this->handler->usedBuffersSize(), 1); + EXPECT_EQ(this->handler->freeBuffersSize(), 0); } -TEST_F(BufferHandlerTest, GetBuffer_ExactSizeMatch) { - auto buffer1 = handler->getBuffer(100, 1.0); - handler->freeBuffer(buffer1); +TYPED_TEST(TypedBufferHandlerTest, GetBuffer_ExactSizeMatch) { + auto buffer1 = this->handler->getBuffer(100, 1.0); + this->handler->freeBuffer(buffer1); - auto buffer2 = handler->getBuffer(100, 1.0); + auto buffer2 = this->handler->getBuffer(100, 1.0); EXPECT_EQ(buffer2->getBufferSize(), 100); - EXPECT_EQ(handler->usedBuffersSize(), 1); - EXPECT_EQ(handler->freeBuffersSize(), 0); + EXPECT_EQ(this->handler->usedBuffersSize(), 1); + EXPECT_EQ(this->handler->freeBuffersSize(), 0); } -TEST_F(BufferHandlerTest, FreeNonExistentBuffer) { - auto buffer = handler->getBuffer(100, 1.0); - auto newBuffer = std::make_shared>(200); +TYPED_TEST(TypedBufferHandlerTest, FreeNonExistentBuffer) { + auto buffer = this->handler->getBuffer(100, 1.0); + auto newBuffer = std::make_shared>(200); - handler->freeBuffer(newBuffer); - EXPECT_EQ(handler->usedBuffersSize(), 1); - EXPECT_EQ(handler->freeBuffersSize(), 0); + this->handler->freeBuffer(newBuffer); + EXPECT_EQ(this->handler->usedBuffersSize(), 1); + EXPECT_EQ(this->handler->freeBuffersSize(), 0); } -TEST_F(BufferHandlerTest, RepeatedAllocateAndFreeCycle) { +TYPED_TEST(TypedBufferHandlerTest, RepeatedAllocateAndFreeCycle) { for (int i = 0; i < 10; ++i) { - auto buffer = handler->getBuffer(100, 1.0); - handler->freeBuffer(buffer); + auto buffer = this->handler->getBuffer(100, 1.0); + this->handler->freeBuffer(buffer); } - - EXPECT_EQ(handler->usedBuffersSize(), 0); - EXPECT_EQ(handler->freeBuffersSize(), 1); + + EXPECT_EQ(this->handler->usedBuffersSize(), 0); + EXPECT_EQ(this->handler->freeBuffersSize(), 1); } -TEST_F(BufferHandlerTest, GetBuffer_ZeroSize) { - auto buffer = handler->getBuffer(0, 1.0); +TYPED_TEST(TypedBufferHandlerTest, GetBuffer_ZeroSize) { + auto buffer = this->handler->getBuffer(0, 1.0); ASSERT_NE(buffer, nullptr); EXPECT_GE(buffer->getBufferSize(), 0); - EXPECT_EQ(handler->usedBuffersSize(), 1); - EXPECT_EQ(handler->freeBuffersSize(), 0); + EXPECT_EQ(this->handler->usedBuffersSize(), 1); + EXPECT_EQ(this->handler->freeBuffersSize(), 0); } -TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_EmptyHandler) { - EXPECT_EQ(handler->getAllocatedSize(), 0); - EXPECT_EQ(handler->getFreeSize(), 0); +TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_EmptyHandler) { + EXPECT_EQ(this->handler->getAllocatedSize(), 0); + EXPECT_EQ(this->handler->getFreeSize(), 0); } -TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_AfterBufferAllocation) { - auto buffer = handler->getBuffer(100, 1.0); - EXPECT_EQ(handler->getAllocatedSize(), 100); - EXPECT_EQ(handler->getFreeSize(), 0); +TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterBufferAllocation) { + auto buffer = this->handler->getBuffer(100, 1.0); + EXPECT_EQ(this->handler->getAllocatedSize(), 100); + EXPECT_EQ(this->handler->getFreeSize(), 0); } -TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeBuffer) { - auto buffer = handler->getBuffer(100, 1.0); - handler->freeBuffer(buffer); +TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeBuffer) { + auto buffer = this->handler->getBuffer(100, 1.0); + this->handler->freeBuffer(buffer); - EXPECT_EQ(handler->getAllocatedSize(), 0); - EXPECT_EQ(handler->getFreeSize(), 100); + EXPECT_EQ(this->handler->getAllocatedSize(), 0); + EXPECT_EQ(this->handler->getFreeSize(), 100); } -TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeAllBuffers) { - auto buffer1 = handler->getBuffer(50, 1.0); - auto buffer2 = handler->getBuffer(100, 1.0); +TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeAllBuffers) { + auto buffer1 = this->handler->getBuffer(50, 1.0); + auto buffer2 = this->handler->getBuffer(100, 1.0); - handler->freeAllBuffers(); + this->handler->freeAllBuffers(); - EXPECT_EQ(handler->getAllocatedSize(), 0); - EXPECT_EQ(handler->getFreeSize(), 150); + EXPECT_EQ(this->handler->getAllocatedSize(), 0); + EXPECT_EQ(this->handler->getFreeSize(), 150); } -TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_AfterDeleteAllBuffers) { - handler->getBuffer(50, 1.0); - handler->getBuffer(100, 1.0); +TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterDeleteAllBuffers) { + this->handler->getBuffer(50, 1.0); + this->handler->getBuffer(100, 1.0); - handler->deleteAllBuffers(); + this->handler->deleteAllBuffers(); - EXPECT_EQ(handler->getAllocatedSize(), 0); - EXPECT_EQ(handler->getFreeSize(), 0); -} - -TEST_F(BufferHandlerTest, GetAllocatedAndFreeSize_ResizeBufferLargerThanAvailable) { - auto smallBuffer = handler->getBuffer(50, 1.0); - handler->freeBuffer(smallBuffer); - - auto largeBuffer = handler->getBuffer(200, 1.0); - - EXPECT_EQ(handler->getAllocatedSize(), 200); - EXPECT_EQ(handler->getFreeSize(), 0); + EXPECT_EQ(this->handler->getAllocatedSize(), 0); + EXPECT_EQ(this->handler->getFreeSize(), 0); } +TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_ResizeBufferLargerThanAvailable) { + auto smallBuffer = this->handler->getBuffer(50, 1.0); + this->handler->freeBuffer(smallBuffer); + auto largeBuffer = this->handler->getBuffer(200, 1.0); + EXPECT_EQ(this->handler->getAllocatedSize(), 200); + EXPECT_EQ(this->handler->getFreeSize(), 0); +} int main(int argc, char* argv[]) { int success = 1; From ee7a8f6c6e2c117acf229c4d6b8c45b2b30101b6 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Fri, 4 Oct 2024 10:27:34 +0200 Subject: [PATCH 08/21] added loggingbufferhandler and unit tests for it --- src/Communicate/CMakeLists.txt | 1 + src/Communicate/LoggingBufferHandler.h | 81 +++++++++++ unit_tests/Communicate/BufferHandler.cpp | 4 - unit_tests/Communicate/CMakeLists.txt | 11 ++ .../Communicate/LoggingBufferHandler.cpp | 126 ++++++++++++++++++ 5 files changed, 219 insertions(+), 4 deletions(-) create mode 100644 src/Communicate/LoggingBufferHandler.h create mode 100644 unit_tests/Communicate/LoggingBufferHandler.cpp diff --git a/src/Communicate/CMakeLists.txt b/src/Communicate/CMakeLists.txt index 9cf7e2240..12c0f3164 100644 --- a/src/Communicate/CMakeLists.txt +++ b/src/Communicate/CMakeLists.txt @@ -7,6 +7,7 @@ set (_SRCS set (_HDRS BufferHandler.h + LoggingBufferHandler.h Archive.h Archive.hpp Buffers.hpp diff --git a/src/Communicate/LoggingBufferHandler.h b/src/Communicate/LoggingBufferHandler.h new file mode 100644 index 000000000..c4a9cdf89 --- /dev/null +++ b/src/Communicate/LoggingBufferHandler.h @@ -0,0 +1,81 @@ +#ifndef IPPL_LOGGING_BUFFER_HANDLER_H +#define IPPL_LOGGING_BUFFER_HANDLER_H + +#include +#include +#include +#include +#include +#include "Communicate/BufferHandler.h" + +struct LogEntry { + std::string methodName; + std::map parameters; + size_t allocatedSize; + size_t freeSize; + std::string memorySpace; + int rank; + std::chrono::time_point timestamp; +}; + +template +class LoggingBufferHandler : public IBufferHandler { +public: + using buffer_type = typename IBufferHandler::buffer_type; + using size_type = typename IBufferHandler::size_type; + + LoggingBufferHandler(std::shared_ptr> handler, int rank) + : handler_(std::move(handler)), rank_(rank) {} + + buffer_type getBuffer(size_type size, double overallocation) override { + auto buffer = handler_->getBuffer(size, overallocation); + logMethod("getBuffer", {{"size", std::to_string(size)}, {"overallocation", std::to_string(overallocation)}}); + return buffer; + } + + void freeBuffer(buffer_type buffer) override { + handler_->freeBuffer(buffer); + logMethod("freeBuffer", {}); + } + + void freeAllBuffers() override { + handler_->freeAllBuffers(); + logMethod("freeAllBuffers", {}); + } + + void deleteAllBuffers() override { + handler_->deleteAllBuffers(); + logMethod("deleteAllBuffers", {}); + } + + size_type getAllocatedSize() const override { + return handler_->getAllocatedSize(); + } + + size_type getFreeSize() const override { + return handler_->getFreeSize(); + } + + const std::vector& getLogs() const { + return logEntries_; + } + +private: + std::shared_ptr> handler_; + std::vector logEntries_; + int rank_; + + void logMethod(const std::string& methodName, const std::map& parameters) { + logEntries_.push_back({ + methodName, + parameters, + handler_->getAllocatedSize(), + handler_->getFreeSize(), + MemorySpace::name(), + rank_, + std::chrono::high_resolution_clock::now() + }); + } +}; + +#endif diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp index 1843b877c..ae4f8756b 100644 --- a/unit_tests/Communicate/BufferHandler.cpp +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -5,10 +5,6 @@ #include "TestUtils.h" #include "gtest/gtest.h" - -//using MemorySpaces = typename ippl::detail::TypeForAllSpaces::unique_memory_spaces; - - template struct VariantToTypes; diff --git a/unit_tests/Communicate/CMakeLists.txt b/unit_tests/Communicate/CMakeLists.txt index 369ec8574..db8ad34d7 100644 --- a/unit_tests/Communicate/CMakeLists.txt +++ b/unit_tests/Communicate/CMakeLists.txt @@ -14,6 +14,9 @@ link_directories ( add_executable (BufferHandler BufferHandler.cpp) gtest_discover_tests(BufferHandler PROPERTIES TEST_DISCOVERY_TIMEOUT 600) +add_executable (LoggingBufferHandler LoggingBufferHandler.cpp) +gtest_discover_tests(LoggingBufferHandler PROPERTIES TEST_DISCOVERY_TIMEOUT 600) + target_link_libraries ( BufferHandler ippl @@ -21,3 +24,11 @@ target_link_libraries ( GTest::gtest_main ${MPI_CXX_LIBRARIES} ) + +target_link_libraries ( + LoggingBufferHandler + ippl + pthread + GTest::gtest_main + ${MPI_CXX_LIBRARIES} +) diff --git a/unit_tests/Communicate/LoggingBufferHandler.cpp b/unit_tests/Communicate/LoggingBufferHandler.cpp new file mode 100644 index 000000000..5b46dfd10 --- /dev/null +++ b/unit_tests/Communicate/LoggingBufferHandler.cpp @@ -0,0 +1,126 @@ +#include "Ippl.h" + +#include "Communicate/LoggingBufferHandler.h" + +#include "Communicate/BufferHandler.h" +#include "gtest/gtest.h" + +template +struct VariantToTypes; + +template +struct VariantToTypes> { + using type = ::testing::Types; +}; + +using MemorySpaces = typename VariantToTypes< + ippl::detail::TypeForAllSpaces::unique_memory_spaces>::type; + +template +class TypedLoggingBufferHandlerTest : public ::testing::Test { +protected: + void SetUp() override { + rank = 0; + this->bufferHandler = std::make_shared>(); + this->loggingHandler = + std::make_shared>(bufferHandler, rank); + } + + int rank; + std::shared_ptr> bufferHandler; + std::shared_ptr> loggingHandler; +}; + +TYPED_TEST_SUITE(TypedLoggingBufferHandlerTest, MemorySpaces); + +template +void compareNumericParameter(const std::string& paramString, T expectedValue, + double tolerance = 1e-6) { + if constexpr (std::is_floating_point::value) { + double actualValue = std::stod(paramString); + EXPECT_NEAR(actualValue, expectedValue, tolerance); + } else if constexpr (std::is_integral::value) { + int actualValue = std::stoi(paramString); + EXPECT_EQ(actualValue, expectedValue); + } else { + FAIL() << "Unsupported type for numeric comparison"; + } +} + +TYPED_TEST(TypedLoggingBufferHandlerTest, GetBufferLogsCorrectly) { + auto buffer = this->loggingHandler->getBuffer(100, 1.5); + const auto& logs = this->loggingHandler->getLogs(); + ASSERT_EQ(logs.size(), 1); + + const auto& entry = logs[0]; + EXPECT_EQ(entry.methodName, "getBuffer"); + + std::string sizeStr = entry.parameters.at("size"); + compareNumericParameter(sizeStr, 100); + + std::string overallocationStr = entry.parameters.at("overallocation"); + compareNumericParameter(overallocationStr, 1.5); + + EXPECT_EQ(entry.allocatedSize, this->bufferHandler->getAllocatedSize()); + EXPECT_EQ(entry.freeSize, this->bufferHandler->getFreeSize()); + EXPECT_EQ(entry.memorySpace, TypeParam::name()); + EXPECT_EQ(entry.rank, this->rank); +} + +TYPED_TEST(TypedLoggingBufferHandlerTest, FreeBufferLogsCorrectly) { + auto buffer = this->loggingHandler->getBuffer(100, 1.0); + this->loggingHandler->freeBuffer(buffer); + + const auto& logs = this->loggingHandler->getLogs(); + ASSERT_EQ(logs.size(), 2); + + const auto& entry = logs[1]; + EXPECT_EQ(entry.methodName, "freeBuffer"); + EXPECT_EQ(entry.allocatedSize, this->bufferHandler->getAllocatedSize()); + EXPECT_EQ(entry.freeSize, this->bufferHandler->getFreeSize()); + EXPECT_EQ(entry.memorySpace, TypeParam::name()); + EXPECT_EQ(entry.rank, this->rank); +} + +TYPED_TEST(TypedLoggingBufferHandlerTest, FreeAllBuffersLogsCorrectly) { + this->loggingHandler->getBuffer(100, 1.0); + this->loggingHandler->getBuffer(200, 1.0); + this->loggingHandler->freeAllBuffers(); + + const auto& logs = this->loggingHandler->getLogs(); + ASSERT_EQ(logs.size(), 3); + + const auto& entry = logs[2]; + EXPECT_EQ(entry.methodName, "freeAllBuffers"); + EXPECT_EQ(entry.allocatedSize, this->bufferHandler->getAllocatedSize()); + EXPECT_EQ(entry.freeSize, this->bufferHandler->getFreeSize()); + EXPECT_EQ(entry.memorySpace, TypeParam::name()); + EXPECT_EQ(entry.rank, this->rank); +} + +TYPED_TEST(TypedLoggingBufferHandlerTest, DeleteAllBuffersLogsCorrectly) { + this->loggingHandler->getBuffer(100, 1.0); + this->loggingHandler->getBuffer(200, 1.0); + this->loggingHandler->deleteAllBuffers(); + + const auto& logs = this->loggingHandler->getLogs(); + ASSERT_EQ(logs.size(), 3); + + const auto& entry = logs[2]; + EXPECT_EQ(entry.methodName, "deleteAllBuffers"); + EXPECT_EQ(entry.allocatedSize, this->bufferHandler->getAllocatedSize()); + EXPECT_EQ(entry.freeSize, this->bufferHandler->getFreeSize()); + EXPECT_EQ(entry.memorySpace, TypeParam::name()); + EXPECT_EQ(entry.rank, this->rank); +} + +int main(int argc, char* argv[]) { + int success = 1; + ippl::initialize(argc, argv); + { + ::testing::InitGoogleTest(&argc, argv); + success = RUN_ALL_TESTS(); + } + ippl::finalize(); + return success; +} From 0963571b56c07cf7e7b720978c192f9da09fc4d2 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 14 Oct 2024 16:55:38 +0200 Subject: [PATCH 09/21] implemented logging of bufferhandler --- src/Communicate/CMakeLists.txt | 4 + src/Communicate/Communicator.h | 11 ++- src/Communicate/CommunicatorLogging.cpp | 122 ++++++++++++++++++++++++ src/Communicate/CommunicatorLogging.hpp | 15 +++ src/Communicate/LogEntry.cpp | 62 ++++++++++++ src/Communicate/LogEntry.h | 38 ++++++++ src/Communicate/LoggingBufferHandler.h | 17 ++-- src/Communicate/PointToPoint.hpp | 3 +- src/Utility/TypeUtils.h | 6 ++ test/field/TestHalo.cpp | 3 + unit_tests/Communicate/CMakeLists.txt | 11 +++ unit_tests/Communicate/LogEntry.cpp | 87 +++++++++++++++++ 12 files changed, 367 insertions(+), 12 deletions(-) create mode 100644 src/Communicate/CommunicatorLogging.cpp create mode 100644 src/Communicate/CommunicatorLogging.hpp create mode 100644 src/Communicate/LogEntry.cpp create mode 100644 src/Communicate/LogEntry.h create mode 100644 unit_tests/Communicate/LogEntry.cpp diff --git a/src/Communicate/CMakeLists.txt b/src/Communicate/CMakeLists.txt index 12c0f3164..8a94ca26f 100644 --- a/src/Communicate/CMakeLists.txt +++ b/src/Communicate/CMakeLists.txt @@ -1,11 +1,14 @@ set (_SRCS Communicator.cpp + CommunicatorLogging.cpp Environment.cpp Buffers.cpp Request.cpp + LogEntry.cpp ) set (_HDRS + LogEntry.h BufferHandler.h LoggingBufferHandler.h Archive.h @@ -25,6 +28,7 @@ set (_HDRS Window.h Window.hpp PointToPoint.hpp + CommunicatorLogging.hpp ) include_directories ( diff --git a/src/Communicate/Communicator.h b/src/Communicate/Communicator.h index a7df873f9..ec9de8d8d 100644 --- a/src/Communicate/Communicator.h +++ b/src/Communicate/Communicator.h @@ -9,6 +9,7 @@ #include #include "Communicate/BufferHandler.h" +#include "Communicate/LoggingBufferHandler.h" #include "Communicate/Request.h" #include "Communicate/Status.h" @@ -135,7 +136,7 @@ namespace ippl { private: template - using buffer_container_type = BufferHandler; + using buffer_container_type = LoggingBufferHandler; using buffer_handler_type = typename detail::ContainerForAllSpaces::type; @@ -192,8 +193,16 @@ namespace ippl { } MPI_Irecv(ar.getBuffer(), msize, MPI_BYTE, src, tag, *comm_m, &request); } + + void printLogs(); private: + std::vector gatherLocalLogs(); + void sendLogsToRank0(const std::vector& localLogs); + std::vector gatherLogsFromAllRanks(const std::vector& localLogs); + void writeLogsToFile(const std::vector& allLogs); + + buffer_handler_type buffer_handlers_m; double defaultOveralloc_m = 1.0; diff --git a/src/Communicate/CommunicatorLogging.cpp b/src/Communicate/CommunicatorLogging.cpp new file mode 100644 index 000000000..308ce3c89 --- /dev/null +++ b/src/Communicate/CommunicatorLogging.cpp @@ -0,0 +1,122 @@ +#include "Communicate/Communicator.h" +#include "Communicate/LogEntry.h" +#include "Communicate/CommunicatorLogging.hpp" + +#include +#include + +namespace ippl { + namespace mpi { + void Communicator::printLogs() { + std::vector localLogs = gatherLocalLogs(); + + std::vector allLogs; + if (rank() == 0) { + allLogs = gatherLogsFromAllRanks(localLogs); + } else { + sendLogsToRank0(localLogs); + } + + if (rank() == 0) { + writeLogsToFile(allLogs); + } + + } + + std::vector Communicator::gatherLocalLogs() { + std::vector localLogs; + + buffer_handlers_m.forAll([&](auto& loggingHandler) { + const auto& logs = loggingHandler.getLogs(); + localLogs.insert(localLogs.end(), logs.begin(), logs.end()); + }); + + return localLogs; + } + + void Communicator::sendLogsToRank0(const std::vector& localLogs) { + std::vector buffer = serializeLogs(localLogs); + + int logSize = buffer.size(); + MPI_Send(&logSize, 1, MPI_INT, 0, 0, MPI_COMM_WORLD); + + MPI_Send(buffer.data(), logSize, MPI_CHAR, 0, 0, MPI_COMM_WORLD); + } + + std::vector Communicator::gatherLogsFromAllRanks(const std::vector& localLogs) { + std::vector allLogs = localLogs; + + int worldSize; + MPI_Comm_size(MPI_COMM_WORLD, &worldSize); + + for (int rank = 1; rank < worldSize; ++rank) { + int logSize; + MPI_Recv(&logSize, 1, MPI_INT, rank, 0, MPI_COMM_WORLD, MPI_STATUS_IGNORE); + + std::vector buffer(logSize); + MPI_Recv(buffer.data(), logSize, MPI_CHAR, rank, 0, MPI_COMM_WORLD, MPI_STATUS_IGNORE); + + std::vector deserializedLogs = deserializeLogs(buffer); + allLogs.insert(allLogs.end(), deserializedLogs.begin(), deserializedLogs.end()); + } + + return allLogs; + } + + + std::vector serializeLogs(const std::vector& logs) { + std::vector buffer; + + for (const auto& logEntry : logs) { + std::vector serializedEntry = logEntry.serialize(); + buffer.insert(buffer.end(), serializedEntry.begin(), serializedEntry.end()); + } + + return buffer; + } + + + std::vector deserializeLogs(const std::vector& buffer) { + std::vector logs; + size_t offset = 0; + + while (offset < buffer.size()) { + LogEntry logEntry = LogEntry::deserialize(buffer, offset); + + logs.push_back(logEntry); + + offset += logEntry.serialize().size(); + } + return logs; + } + + void Communicator::writeLogsToFile(const std::vector& allLogs) { + std::ofstream logFile("log_entries.csv"); + + logFile << "Timestamp,Method,Rank,MemorySpace,AllocatedSize,FreeSize,Parameters\n"; + + for (const auto& log : allLogs) { + auto timestamp = std::chrono::duration_cast( + log.timestamp.time_since_epoch()) + .count(); + + logFile << timestamp << "," << log.methodName << "," << log.rank << "," << log.memorySpace << "," + << log.allocatedSize << "," << log.freeSize; + + logFile << ",\""; + bool first = true; + for (const auto& [key, value] : log.parameters) { + if (!first) { + logFile << "; "; // Separate key-value pairs with a semicolon + } + logFile << key << ": " << value; + first = false; + } + logFile << "\"\n"; // End of the line + } + + logFile.close(); + } + + } // namespace mpi +} // namespace ippl diff --git a/src/Communicate/CommunicatorLogging.hpp b/src/Communicate/CommunicatorLogging.hpp new file mode 100644 index 000000000..5746c3df9 --- /dev/null +++ b/src/Communicate/CommunicatorLogging.hpp @@ -0,0 +1,15 @@ +#ifndef IPPL_COMMUNICATOR_LOGGING_HPP +#define IPPL_COMMUNICATOR_LOGGING_HPP + +#include + +#include "Communicate/LogEntry.h" + +namespace ippl { + namespace mpi { + std::vector serializeLogs(const std::vector& logs); + std::vector deserializeLogs(const std::vector& buffer); + } // namespace mpi +} // namespace ippl + +#endif diff --git a/src/Communicate/LogEntry.cpp b/src/Communicate/LogEntry.cpp new file mode 100644 index 000000000..a33d38aaa --- /dev/null +++ b/src/Communicate/LogEntry.cpp @@ -0,0 +1,62 @@ +#include "Communicate/LogEntry.h" + +void serializeString(std::vector& buffer, const std::string& str) { + size_t length = str.size(); + serializeBasicType(buffer, length); // First, serialize the length of the string + buffer.insert(buffer.end(), str.begin(), str.end()); // Then, serialize the string itself +} + +std::string deserializeString(const std::vector& buffer, size_t& offset) { + size_t length = deserializeBasicType(buffer, offset); // Get the length of the string + std::string str(buffer.begin() + offset, buffer.begin() + offset + length); // Extract the string + offset += length; + return str; +} + +std::vector LogEntry::serialize() const { + std::vector buffer; + + serializeString(buffer, methodName); + serializeBasicType(buffer, allocatedSize); + serializeBasicType(buffer, freeSize); + serializeString(buffer, memorySpace); + serializeBasicType(buffer, rank); + + // Serialize the timestamp (as duration since epoch) + auto duration = timestamp.time_since_epoch().count(); + serializeBasicType(buffer, duration); + + size_t mapSize = parameters.size(); + serializeBasicType(buffer, mapSize); + for (const auto& pair : parameters) { + serializeString(buffer, pair.first); + serializeString(buffer, pair.second); + } + + return buffer; +} + +LogEntry LogEntry::deserialize(const std::vector& buffer, size_t offset) { + LogEntry entry; + size_t current_pos = offset; + + entry.methodName = deserializeString(buffer, current_pos); + entry.allocatedSize = deserializeBasicType(buffer, current_pos); + entry.freeSize = deserializeBasicType(buffer, current_pos); + entry.memorySpace = deserializeString(buffer, current_pos); + entry.rank = deserializeBasicType(buffer, current_pos); + + auto duration = deserializeBasicType(buffer, current_pos); + entry.timestamp = std::chrono::time_point( + std::chrono::high_resolution_clock::duration(duration) + ); + + size_t mapSize = deserializeBasicType(buffer, current_pos); + for (size_t i = 0; i < mapSize; ++i) { + std::string key = deserializeString(buffer, current_pos); + std::string value = deserializeString(buffer, current_pos); + entry.parameters[key] = value; + } + + return entry; +} diff --git a/src/Communicate/LogEntry.h b/src/Communicate/LogEntry.h new file mode 100644 index 000000000..14a79c361 --- /dev/null +++ b/src/Communicate/LogEntry.h @@ -0,0 +1,38 @@ +#ifndef IPPL_LOGENTRY_H +#define IPPL_LOGENTRY_H + +#include +#include +#include +#include +#include + +struct LogEntry { + std::string methodName; + std::map parameters; + size_t allocatedSize; + size_t freeSize; + std::string memorySpace; + int rank; + std::chrono::time_point timestamp; + + std::vector serialize() const; + static LogEntry deserialize(const std::vector& buffer, size_t offset = 0); +}; + +template +void serializeBasicType(std::vector& buffer, const T& value) { + size_t size = sizeof(T); + buffer.resize(buffer.size() + size); + std::memcpy(buffer.data() + buffer.size() - size, &value, size); +} + +template +T deserializeBasicType(const std::vector& buffer, size_t& offset) { + T value; + std::memcpy(&value, buffer.data() + offset, sizeof(T)); + offset += sizeof(T); + return value; +} + +#endif diff --git a/src/Communicate/LoggingBufferHandler.h b/src/Communicate/LoggingBufferHandler.h index c4a9cdf89..a10580bfd 100644 --- a/src/Communicate/LoggingBufferHandler.h +++ b/src/Communicate/LoggingBufferHandler.h @@ -7,16 +7,7 @@ #include #include #include "Communicate/BufferHandler.h" - -struct LogEntry { - std::string methodName; - std::map parameters; - size_t allocatedSize; - size_t freeSize; - std::string memorySpace; - int rank; - std::chrono::time_point timestamp; -}; +#include "Communicate/LogEntry.h" template class LoggingBufferHandler : public IBufferHandler { @@ -27,6 +18,11 @@ class LoggingBufferHandler : public IBufferHandler { LoggingBufferHandler(std::shared_ptr> handler, int rank) : handler_(std::move(handler)), rank_(rank) {} + LoggingBufferHandler() { + handler_ = std::make_shared>(); + MPI_Comm_rank(MPI_COMM_WORLD, &rank_); + } + buffer_type getBuffer(size_type size, double overallocation) override { auto buffer = handler_->getBuffer(size, overallocation); logMethod("getBuffer", {{"size", std::to_string(size)}, {"overallocation", std::to_string(overallocation)}}); @@ -57,6 +53,7 @@ class LoggingBufferHandler : public IBufferHandler { } const std::vector& getLogs() const { + std::cout << logEntries_.size() << std::endl; return logEntries_; } diff --git a/src/Communicate/PointToPoint.hpp b/src/Communicate/PointToPoint.hpp index 830c16385..16393d63c 100644 --- a/src/Communicate/PointToPoint.hpp +++ b/src/Communicate/PointToPoint.hpp @@ -25,6 +25,7 @@ namespace ippl { template void Communicator::recv(T* output, int count, int source, int tag, Status& status) { MPI_Datatype type = get_mpi_datatype(*output); + std::cout << "recv called!" << std::endl; MPI_Recv(output, count, type, source, tag, comm_m, &status); } @@ -42,7 +43,7 @@ namespace ippl { void Communicator::isend(const T* buffer, int count, int dest, int tag, Request& request) { MPI_Datatype type = get_mpi_datatype(*buffer); - MPI_Isend(buffer, count, type, dest, tag, *comm_m, request); + MPI_Isend(buffer, count, type, dest, tag, comm_m, request); } template diff --git a/src/Utility/TypeUtils.h b/src/Utility/TypeUtils.h index 0d86deef8..a49474bca 100644 --- a/src/Utility/TypeUtils.h +++ b/src/Utility/TypeUtils.h @@ -410,6 +410,12 @@ namespace ippl { using container_type = MultispaceContainer; using type = typename TypeForAllSpaces::memory_spaces_type; + + // Static factory function that takes a lambda to initialize each memory space + template + static type createContainer(Functor&& initFunc) { + return type{std::forward(initFunc)}; + } }; /*! diff --git a/test/field/TestHalo.cpp b/test/field/TestHalo.cpp index 8b78bc010..b68db6abd 100644 --- a/test/field/TestHalo.cpp +++ b/test/field/TestHalo.cpp @@ -144,6 +144,9 @@ int main(int argc, char* argv[]) { IpplTimings::stopTimer(mainTimer); IpplTimings::print(); IpplTimings::print(std::string("timing.dat")); + + layout.comm.printLogs(); + } ippl::finalize(); diff --git a/unit_tests/Communicate/CMakeLists.txt b/unit_tests/Communicate/CMakeLists.txt index db8ad34d7..df21a5e5d 100644 --- a/unit_tests/Communicate/CMakeLists.txt +++ b/unit_tests/Communicate/CMakeLists.txt @@ -17,6 +17,9 @@ gtest_discover_tests(BufferHandler PROPERTIES TEST_DISCOVERY_TIMEOUT 600) add_executable (LoggingBufferHandler LoggingBufferHandler.cpp) gtest_discover_tests(LoggingBufferHandler PROPERTIES TEST_DISCOVERY_TIMEOUT 600) +add_executable (LogEntry LogEntry.cpp) +gtest_discover_tests(LogEntry PROPERTIES TEST_DISCOVERY_TIMEOUT 600) + target_link_libraries ( BufferHandler ippl @@ -32,3 +35,11 @@ target_link_libraries ( GTest::gtest_main ${MPI_CXX_LIBRARIES} ) + +target_link_libraries ( + LogEntry + ippl + pthread + GTest::gtest_main + ${MPI_CXX_LIBRARIES} +) diff --git a/unit_tests/Communicate/LogEntry.cpp b/unit_tests/Communicate/LogEntry.cpp new file mode 100644 index 000000000..4ad27eac0 --- /dev/null +++ b/unit_tests/Communicate/LogEntry.cpp @@ -0,0 +1,87 @@ +#include "Ippl.h" + +#include "Communicate/LogEntry.h" + +#include "TestUtils.h" +#include "gtest/gtest.h" + +LogEntry createSampleLogEntry() { + LogEntry logEntry; + logEntry.methodName = "TestMethod"; + logEntry.allocatedSize = 1024; + logEntry.freeSize = 512; + logEntry.memorySpace = "HostSpace"; + logEntry.rank = 1; + logEntry.timestamp = std::chrono::high_resolution_clock::now(); + + logEntry.parameters["overallocation"] = "1.5"; + logEntry.parameters["buffer_size"] = "2048"; + + return logEntry; +} + +TEST(LogEntryTest, Serialize) { + LogEntry logEntry = createSampleLogEntry(); + + std::vector buffer; + buffer = logEntry.serialize(); + + EXPECT_GT(buffer.size(), 0); +} + +TEST(LogEntryTest, Deserialize) { + LogEntry logEntry = createSampleLogEntry(); + + std::vector buffer; + buffer = logEntry.serialize(); + + LogEntry deserializedLogEntry = LogEntry::deserialize(buffer); + + EXPECT_EQ(deserializedLogEntry.methodName, logEntry.methodName); + EXPECT_EQ(deserializedLogEntry.allocatedSize, logEntry.allocatedSize); + EXPECT_EQ(deserializedLogEntry.freeSize, logEntry.freeSize); + EXPECT_EQ(deserializedLogEntry.memorySpace, logEntry.memorySpace); + EXPECT_EQ(deserializedLogEntry.rank, logEntry.rank); + + EXPECT_EQ(deserializedLogEntry.parameters.size(), logEntry.parameters.size()); + EXPECT_EQ(deserializedLogEntry.parameters.at("overallocation"), logEntry.parameters.at("overallocation")); + EXPECT_EQ(deserializedLogEntry.parameters.at("buffer_size"), logEntry.parameters.at("buffer_size")); + + auto originalTime = logEntry.timestamp.time_since_epoch().count(); + auto deserializedTime = deserializedLogEntry.timestamp.time_since_epoch().count(); + EXPECT_EQ(originalTime, deserializedTime); +} + +TEST(LogEntryTest, RoundTripSerialization) { + LogEntry logEntry = createSampleLogEntry(); + + std::vector buffer; + buffer = logEntry.serialize(); + + LogEntry deserializedLogEntry = LogEntry::deserialize(buffer); + + EXPECT_EQ(deserializedLogEntry.methodName, logEntry.methodName); + EXPECT_EQ(deserializedLogEntry.allocatedSize, logEntry.allocatedSize); + EXPECT_EQ(deserializedLogEntry.freeSize, logEntry.freeSize); + EXPECT_EQ(deserializedLogEntry.memorySpace, logEntry.memorySpace); + EXPECT_EQ(deserializedLogEntry.rank, logEntry.rank); + + EXPECT_EQ(deserializedLogEntry.parameters.size(), logEntry.parameters.size()); + EXPECT_EQ(deserializedLogEntry.parameters.at("overallocation"), logEntry.parameters.at("overallocation")); + EXPECT_EQ(deserializedLogEntry.parameters.at("buffer_size"), logEntry.parameters.at("buffer_size")); + + auto originalTime = logEntry.timestamp.time_since_epoch().count(); + auto deserializedTime = deserializedLogEntry.timestamp.time_since_epoch().count(); + EXPECT_EQ(originalTime, deserializedTime); +} + +int main(int argc, char* argv[]) { + int success = 1; + ippl::initialize(argc, argv); + { + ::testing::InitGoogleTest(&argc, argv); + success = RUN_ALL_TESTS(); + } + ippl::finalize(); + return success; +} From 6f59a03bc80f4130bff958d5c5813f632bd6f63b Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 14 Oct 2024 17:42:04 +0200 Subject: [PATCH 10/21] removed unnecessary comments --- src/Communicate/CommunicatorLogging.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Communicate/CommunicatorLogging.cpp b/src/Communicate/CommunicatorLogging.cpp index 308ce3c89..63343a368 100644 --- a/src/Communicate/CommunicatorLogging.cpp +++ b/src/Communicate/CommunicatorLogging.cpp @@ -107,12 +107,12 @@ namespace ippl { bool first = true; for (const auto& [key, value] : log.parameters) { if (!first) { - logFile << "; "; // Separate key-value pairs with a semicolon + logFile << "; "; } logFile << key << ": " << value; first = false; } - logFile << "\"\n"; // End of the line + logFile << "\"\n"; } logFile.close(); From c5221d2bd0e7db131f3df6b63fd20420f3205df5 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 28 Oct 2024 09:59:13 +0100 Subject: [PATCH 11/21] attempted fix for memory space issue --- unit_tests/Communicate/BufferHandler.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp index ae4f8756b..b21fba34e 100644 --- a/unit_tests/Communicate/BufferHandler.cpp +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -1,19 +1,18 @@ #include "Ippl.h" +#include "Utility/TypeUtils.h" #include "Communicate/BufferHandler.h" #include "TestUtils.h" #include "gtest/gtest.h" -template -struct VariantToTypes; +using MemorySpaces = ippl::detail::TypeForAllSpaces<::testing::Types>::memory_spaces_type; -template -struct VariantToTypes> { - using type = ::testing::Types; -}; +template +void debugType() { + std::cout << __PRETTY_FUNCTION__ << '\n'; +} -using MemorySpaces = typename VariantToTypes::unique_memory_spaces>::type; template @@ -187,6 +186,8 @@ int main(int argc, char* argv[]) { { ::testing::InitGoogleTest(&argc, argv); success = RUN_ALL_TESTS(); + + debugType(); } ippl::finalize(); return success; From edeb45087108f9fc7b1d7299028ec202ff1e1c31 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 28 Oct 2024 10:16:55 +0100 Subject: [PATCH 12/21] attempted fix for memory space issue for logging as well --- unit_tests/Communicate/LoggingBufferHandler.cpp | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/unit_tests/Communicate/LoggingBufferHandler.cpp b/unit_tests/Communicate/LoggingBufferHandler.cpp index 5b46dfd10..e72ee5eb0 100644 --- a/unit_tests/Communicate/LoggingBufferHandler.cpp +++ b/unit_tests/Communicate/LoggingBufferHandler.cpp @@ -5,16 +5,7 @@ #include "Communicate/BufferHandler.h" #include "gtest/gtest.h" -template -struct VariantToTypes; - -template -struct VariantToTypes> { - using type = ::testing::Types; -}; - -using MemorySpaces = typename VariantToTypes< - ippl::detail::TypeForAllSpaces::unique_memory_spaces>::type; +using MemorySpaces = ippl::detail::TypeForAllSpaces<::testing::Types>::memory_spaces_type; template class TypedLoggingBufferHandlerTest : public ::testing::Test { From bb2171d59d87f2e18d50d2dc3e41c34d76113e22 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 28 Oct 2024 15:25:01 +0100 Subject: [PATCH 13/21] made communication for logging run over the communicator class and fixed some bugs in the pointtopoint mpi wrappers for blocking communication --- src/Communicate/CommunicatorLogging.cpp | 15 +++++++-------- src/Communicate/PointToPoint.hpp | 7 +++---- unit_tests/Communicate/BufferHandler.cpp | 1 - 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/Communicate/CommunicatorLogging.cpp b/src/Communicate/CommunicatorLogging.cpp index 63343a368..99668ae84 100644 --- a/src/Communicate/CommunicatorLogging.cpp +++ b/src/Communicate/CommunicatorLogging.cpp @@ -38,23 +38,22 @@ namespace ippl { std::vector buffer = serializeLogs(localLogs); int logSize = buffer.size(); - MPI_Send(&logSize, 1, MPI_INT, 0, 0, MPI_COMM_WORLD); - MPI_Send(buffer.data(), logSize, MPI_CHAR, 0, 0, MPI_COMM_WORLD); + this->send(logSize, 1, 0, 0); + this->send(buffer.data(), logSize, 0, 0); } std::vector Communicator::gatherLogsFromAllRanks(const std::vector& localLogs) { std::vector allLogs = localLogs; - int worldSize; - MPI_Comm_size(MPI_COMM_WORLD, &worldSize); - - for (int rank = 1; rank < worldSize; ++rank) { + for (int rank = 1; rank < size_m; ++rank) { int logSize; - MPI_Recv(&logSize, 1, MPI_INT, rank, 0, MPI_COMM_WORLD, MPI_STATUS_IGNORE); + Status status; + + this->recv(logSize, 1, rank, 0, status); std::vector buffer(logSize); - MPI_Recv(buffer.data(), logSize, MPI_CHAR, rank, 0, MPI_COMM_WORLD, MPI_STATUS_IGNORE); + this->recv(buffer.data(), logSize, rank, 0, status); std::vector deserializedLogs = deserializeLogs(buffer); allLogs.insert(allLogs.end(), deserializedLogs.begin(), deserializedLogs.end()); diff --git a/src/Communicate/PointToPoint.hpp b/src/Communicate/PointToPoint.hpp index 16393d63c..1c30f5f70 100644 --- a/src/Communicate/PointToPoint.hpp +++ b/src/Communicate/PointToPoint.hpp @@ -12,9 +12,9 @@ namespace ippl { template void Communicator::send(const T* buf, int count, int dest, int tag) { - MPI_Datatype type = get_mpi_datatype(buf); + MPI_Datatype type = get_mpi_datatype(*buf); - MPI_Send(&buf, count, type, dest, tag, comm_m); + MPI_Send(buf, count, type, dest, tag, *comm_m); } template @@ -25,9 +25,8 @@ namespace ippl { template void Communicator::recv(T* output, int count, int source, int tag, Status& status) { MPI_Datatype type = get_mpi_datatype(*output); - std::cout << "recv called!" << std::endl; - MPI_Recv(output, count, type, source, tag, comm_m, &status); + MPI_Recv(output, count, type, source, tag, *comm_m, status); } /* diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp index b21fba34e..39ada92d8 100644 --- a/unit_tests/Communicate/BufferHandler.cpp +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -187,7 +187,6 @@ int main(int argc, char* argv[]) { ::testing::InitGoogleTest(&argc, argv); success = RUN_ALL_TESTS(); - debugType(); } ippl::finalize(); return success; From 79bfe25740902510f0895616ad6acf068f17b935 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 28 Oct 2024 16:45:32 +0100 Subject: [PATCH 14/21] added freeing of memory after communication in ffropenpoissonsolver and with particles --- alpine/LandauDamping.cpp | 1 + src/FFT/FFT.h | 2 +- src/Particle/ParticleSpatialLayout.hpp | 2 ++ src/PoissonSolvers/FFTOpenPoissonSolver.hpp | 6 ++++++ test/particle/TestScatter.cpp | 1 + 5 files changed, 11 insertions(+), 1 deletion(-) diff --git a/alpine/LandauDamping.cpp b/alpine/LandauDamping.cpp index 5322afd50..9fc7af057 100644 --- a/alpine/LandauDamping.cpp +++ b/alpine/LandauDamping.cpp @@ -81,6 +81,7 @@ int main(int argc, char* argv[]) { IpplTimings::stopTimer(mainTimer); IpplTimings::print(); IpplTimings::print(std::string("timing.dat")); + ippl::Comm->printLogs(); } ippl::finalize(); diff --git a/src/FFT/FFT.h b/src/FFT/FFT.h index 7b75fca6c..711bafa40 100644 --- a/src/FFT/FFT.h +++ b/src/FFT/FFT.h @@ -89,7 +89,7 @@ namespace ippl { }; #endif -#if !defined(KOKKOS_ENABLE_CUDA) && !defined(Heffte_ENABLE_MKL) && !defined(Heffte_ENABLE_FFTW) +#if !defined(Heffte_ENABLE_MKL) && !defined(Heffte_ENABLE_FFTW) /** * Use heFFTe's inbuilt 1D fft computation on CPUs if no * vendor specific or optimized backend is found diff --git a/src/Particle/ParticleSpatialLayout.hpp b/src/Particle/ParticleSpatialLayout.hpp index 2d15dfdbe..8da689460 100644 --- a/src/Particle/ParticleSpatialLayout.hpp +++ b/src/Particle/ParticleSpatialLayout.hpp @@ -150,6 +150,8 @@ namespace ippl { if (requests.size() > 0) { MPI_Waitall(requests.size(), requests.data(), MPI_STATUSES_IGNORE); } + Comm->freeAllBuffers(); + IpplTimings::stopTimer(sendTimer); IpplTimings::stopTimer(ParticleUpdateTimer); diff --git a/src/PoissonSolvers/FFTOpenPoissonSolver.hpp b/src/PoissonSolvers/FFTOpenPoissonSolver.hpp index b842449a7..63f8afa54 100644 --- a/src/PoissonSolvers/FFTOpenPoissonSolver.hpp +++ b/src/PoissonSolvers/FFTOpenPoissonSolver.hpp @@ -584,6 +584,7 @@ namespace ippl { if (requests.size() > 0) { MPI_Waitall(requests.size(), requests.data(), MPI_STATUSES_IGNORE); } + ippl::Comm->freeAllBuffers(); } else { Kokkos::parallel_for( @@ -710,6 +711,7 @@ namespace ippl { if (requests.size() > 0) { MPI_Waitall(requests.size(), requests.data(), MPI_STATUSES_IGNORE); } + ippl::Comm->freeAllBuffers(); } else { Kokkos::parallel_for( @@ -866,6 +868,7 @@ namespace ippl { if (requests.size() > 0) { MPI_Waitall(requests.size(), requests.data(), MPI_STATUSES_IGNORE); } + ippl::Comm->freeAllBuffers(); } else { Kokkos::parallel_for( @@ -1023,6 +1026,7 @@ namespace ippl { if (requests.size() > 0) { MPI_Waitall(requests.size(), requests.data(), MPI_STATUSES_IGNORE); } + ippl::Comm->freeAllBuffers(); } else { Kokkos::parallel_for( @@ -1777,6 +1781,7 @@ namespace ippl { if (requests.size() > 0) { MPI_Waitall(requests.size(), requests.data(), MPI_STATUSES_IGNORE); } + ippl::Comm->freeAllBuffers(); }; // CommunicateVico for DCT_VICO (2N+1 to 2N) @@ -2182,5 +2187,6 @@ namespace ippl { if (requests.size() > 0) { MPI_Waitall(requests.size(), requests.data(), MPI_STATUSES_IGNORE); } + ippl::Comm->freeAllBuffers(); }; } // namespace ippl diff --git a/test/particle/TestScatter.cpp b/test/particle/TestScatter.cpp index d92f6a9b1..d7500352b 100644 --- a/test/particle/TestScatter.cpp +++ b/test/particle/TestScatter.cpp @@ -102,6 +102,7 @@ int main(int argc, char* argv[]) { } catch (const std::exception& e) { std::cout << e.what() << std::endl; } + ippl::Comm->printLogs(); } ippl::finalize(); From daf23b93226756c51d4a0fb9cbe4522f3254d162 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 28 Oct 2024 21:25:25 +0100 Subject: [PATCH 15/21] removed parameters that were used for tag computation and aren't used anymore --- src/Particle/ParticleBase.h | 4 +- src/Particle/ParticleBase.hpp | 5 +- src/Particle/ParticleSpatialLayout.hpp | 7 +- src/PoissonSolvers/FFTOpenPoissonSolver.hpp | 86 ++++++++++----------- 4 files changed, 47 insertions(+), 55 deletions(-) diff --git a/src/Particle/ParticleBase.h b/src/Particle/ParticleBase.h index d261f11c3..97017c708 100644 --- a/src/Particle/ParticleBase.h +++ b/src/Particle/ParticleBase.h @@ -289,7 +289,7 @@ namespace ippl { * @param hash a hash view indicating which particles need to be sent to which rank */ template - void sendToRank(int rank, int tag, int sendNum, std::vector& requests, + void sendToRank(int rank, int tag, std::vector& requests, const HashType& hash); /*! @@ -299,7 +299,7 @@ namespace ippl { * @param recvNum the number of messages already received (to distinguish the buffers) * @param nRecvs the number of particles to receive */ - void recvFromRank(int rank, int tag, int recvNum, size_type nRecvs); + void recvFromRank(int rank, int tag, size_type nRecvs); /*! * Serialize to do MPI calls. diff --git a/src/Particle/ParticleBase.hpp b/src/Particle/ParticleBase.hpp index dc937b3bb..adf4b9204 100644 --- a/src/Particle/ParticleBase.hpp +++ b/src/Particle/ParticleBase.hpp @@ -264,7 +264,7 @@ namespace ippl { template template - void ParticleBase::sendToRank(int rank, int tag, int sendNum, + void ParticleBase::sendToRank(int rank, int tag, std::vector& requests, const HashType& hash) { size_type nSends = hash.size(); @@ -288,8 +288,7 @@ namespace ippl { } template - void ParticleBase::recvFromRank(int rank, int tag, int recvNum, - size_type nRecvs) { + void ParticleBase::recvFromRank(int rank, int tag, size_type nRecvs) { detail::runForAllSpaces([&]() { size_type bufSize = packedSize(nRecvs); if (bufSize == 0) { diff --git a/src/Particle/ParticleSpatialLayout.hpp b/src/Particle/ParticleSpatialLayout.hpp index 8da689460..b926f48f9 100644 --- a/src/Particle/ParticleSpatialLayout.hpp +++ b/src/Particle/ParticleSpatialLayout.hpp @@ -114,13 +114,12 @@ namespace ippl { int tag = Comm->next_tag(mpi::tag::P_SPATIAL_LAYOUT, mpi::tag::P_LAYOUT_CYCLE); - int sends = 0; for (int rank = 0; rank < nRanks; ++rank) { if (nSends[rank] > 0) { hash_type hash("hash", nSends[rank]); fillHash(rank, ranks, hash); - pc.sendToRank(rank, tag, sends++, requests, hash); + pc.sendToRank(rank, tag, requests, hash); } } IpplTimings::stopTimer(sendTimer); @@ -137,10 +136,10 @@ namespace ippl { static IpplTimings::TimerRef recvTimer = IpplTimings::getTimer("particleRecv"); IpplTimings::startTimer(recvTimer); // 4th step - int recvs = 0; + for (int rank = 0; rank < nRanks; ++rank) { if (nRecvs[rank] > 0) { - pc.recvFromRank(rank, tag, recvs++, nRecvs[rank]); + pc.recvFromRank(rank, tag, nRecvs[rank]); } } IpplTimings::stopTimer(recvTimer); diff --git a/src/PoissonSolvers/FFTOpenPoissonSolver.hpp b/src/PoissonSolvers/FFTOpenPoissonSolver.hpp index 63f8afa54..c4c4dcc9c 100644 --- a/src/PoissonSolvers/FFTOpenPoissonSolver.hpp +++ b/src/PoissonSolvers/FFTOpenPoissonSolver.hpp @@ -107,7 +107,7 @@ void unpack(const ippl::NDIndex<3> intersect, } template -void solver_send(int BUF_MSG, int TAG, int id, int i, const ippl::NDIndex intersection, +void solver_send(int TAG, int id, int i, const ippl::NDIndex intersection, const ippl::NDIndex ldom, int nghost, Kokkos::View& view, ippl::detail::FieldBufferData& fd, std::vector& requests) { using memory_space = typename Kokkos::View::memory_space; @@ -128,15 +128,14 @@ void solver_send(int BUF_MSG, int TAG, int id, int i, const ippl::NDIndex i } template -void solver_recv(int BUF_MSG, int TAG, int id, int i, const ippl::NDIndex intersection, +void solver_recv(int TAG, int id, int i, const ippl::NDIndex intersection, const ippl::NDIndex ldom, int nghost, Kokkos::View& view, ippl::detail::FieldBufferData& fd, bool x = false, bool y = false, bool z = false) { using memory_space = typename Kokkos::View::memory_space; ippl::mpi::Communicator::size_type nrecvs; - const int myRank = ippl::Comm->rank(); - nrecvs = intersection.size(); + nrecvs = intersection.size(); // Buffer message indicates the domain intersection (x, y, z, xy, yz, xz, xyz). ippl::mpi::Communicator::buffer_type buf = @@ -554,14 +553,13 @@ namespace ippl { if (lDomains2[i].touches(ldom1)) { auto intersection = lDomains2[i].intersect(ldom1); - solver_send(mpi::tag::SOLVER_SEND, mpi::tag::OPEN_SOLVER, 0, i, intersection, + solver_send(mpi::tag::OPEN_SOLVER, 0, i, intersection, ldom1, nghost1, view1, fd_m, requests); } } // receive const auto& lDomains1 = layout_mp->getHostLocalDomains(); - int myRank = Comm->rank(); for (int i = 0; i < ranks; ++i) { if (lDomains1[i].touches(ldom2)) { @@ -570,8 +568,7 @@ namespace ippl { mpi::Communicator::size_type nrecvs; nrecvs = intersection.size(); - buffer_type buf = - Comm->getBuffer(nrecvs); + buffer_type buf = Comm->getBuffer(nrecvs); Comm->recv(i, mpi::tag::OPEN_SOLVER, fd_m, *buf, nrecvs * sizeof(Trhs), nrecvs); buf->resetReadPos(); @@ -681,14 +678,13 @@ namespace ippl { if (lDomains1[i].touches(ldom2)) { auto intersection = lDomains1[i].intersect(ldom2); - solver_send(mpi::tag::SOLVER_SEND, mpi::tag::OPEN_SOLVER, 0, i, + solver_send(mpi::tag::OPEN_SOLVER, 0, i, intersection, ldom2, nghost2, view2, fd_m, requests); } } // receive const auto& lDomains2 = layout2_m->getHostLocalDomains(); - int myRank = Comm->rank(); for (int i = 0; i < ranks; ++i) { if (ldom1.touches(lDomains2[i])) { @@ -838,14 +834,13 @@ namespace ippl { if (lDomains1[i].touches(ldom2)) { auto intersection = lDomains1[i].intersect(ldom2); - solver_send(mpi::tag::SOLVER_SEND, mpi::tag::OPEN_SOLVER, 0, i, + solver_send(mpi::tag::OPEN_SOLVER, 0, i, intersection, ldom2, nghost2, view2, fd_m, requests); } } // receive const auto& lDomains2 = layout2_m->getHostLocalDomains(); - int myRank = Comm->rank(); for (int i = 0; i < ranks; ++i) { if (ldom1.touches(lDomains2[i])) { @@ -996,14 +991,13 @@ namespace ippl { if (lDomains1[i].touches(ldom2)) { auto intersection = lDomains1[i].intersect(ldom2); - solver_send(mpi::tag::SOLVER_SEND, mpi::tag::OPEN_SOLVER, 0, i, + solver_send(mpi::tag::OPEN_SOLVER, 0, i, intersection, ldom2, nghost2, view2, fd_m, requests); } } // receive const auto& lDomains2 = layout2_m->getHostLocalDomains(); - int myRank = Comm->rank(); for (int i = 0; i < ranks; ++i) { if (ldom1.touches(lDomains2[i])) { @@ -1444,7 +1438,7 @@ namespace ippl { if (ldom_g.touches(intersection)) { intersection = intersection.intersect(ldom_g); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 0, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 0, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1462,7 +1456,7 @@ namespace ippl { if (ldom_g.touches(domain4)) { intersection = ldom_g.intersect(domain4); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 1, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 1, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1480,7 +1474,7 @@ namespace ippl { if (ldom_g.touches(domain4)) { intersection = ldom_g.intersect(domain4); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 2, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 2, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1498,7 +1492,7 @@ namespace ippl { if (ldom_g.touches(domain4)) { intersection = ldom_g.intersect(domain4); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 3, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 3, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1518,7 +1512,7 @@ namespace ippl { if (ldom_g.touches(domain4)) { intersection = ldom_g.intersect(domain4); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 4, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 4, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1537,7 +1531,7 @@ namespace ippl { if (ldom_g.touches(domain4)) { intersection = ldom_g.intersect(domain4); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 5, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 5, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1556,7 +1550,7 @@ namespace ippl { if (ldom_g.touches(domain4)) { intersection = ldom_g.intersect(domain4); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 6, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 6, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1577,7 +1571,7 @@ namespace ippl { if (ldom_g.touches(domain4)) { intersection = ldom_g.intersect(domain4); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 7, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 7, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1591,7 +1585,7 @@ namespace ippl { if (lDomains4[i].touches(intersection)) { intersection = intersection.intersect(lDomains4[i]); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 0, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 0, i, intersection, ldom, nghost, view, fd_m); } } @@ -1614,7 +1608,7 @@ namespace ippl { intersection = intersection.intersect(domain4); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 1, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 1, i, intersection, ldom, nghost, view, fd_m, true, false, false); } } @@ -1637,7 +1631,7 @@ namespace ippl { intersection = intersection.intersect(domain4); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 2, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 2, i, intersection, ldom, nghost, view, fd_m, false, true, false); } } @@ -1660,7 +1654,7 @@ namespace ippl { intersection = intersection.intersect(domain4); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 3, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 3, i, intersection, ldom, nghost, view, fd_m, false, false, true); } } @@ -1687,7 +1681,7 @@ namespace ippl { intersection = intersection.intersect(domain4); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 4, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 4, i, intersection, ldom, nghost, view, fd_m, true, true, false); } } @@ -1714,7 +1708,7 @@ namespace ippl { intersection = intersection.intersect(domain4); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 5, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 5, i, intersection, ldom, nghost, view, fd_m, false, true, true); } } @@ -1741,7 +1735,7 @@ namespace ippl { intersection = intersection.intersect(domain4); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 6, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 6, i, intersection, ldom, nghost, view, fd_m, true, false, true); } } @@ -1772,7 +1766,7 @@ namespace ippl { intersection = intersection.intersect(domain4); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 7, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 7, i, intersection, ldom, nghost, view, fd_m, true, true, true); } } @@ -1847,7 +1841,7 @@ namespace ippl { if (ldom_g.touches(intersection)) { intersection = intersection.intersect(ldom_g); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 0, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 0, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1865,7 +1859,7 @@ namespace ippl { if (ldom_g.touches(domain2n1)) { intersection = ldom_g.intersect(domain2n1); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 1, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 1, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1883,7 +1877,7 @@ namespace ippl { if (ldom_g.touches(domain2n1)) { intersection = ldom_g.intersect(domain2n1); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 2, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 2, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1901,7 +1895,7 @@ namespace ippl { if (ldom_g.touches(domain2n1)) { intersection = ldom_g.intersect(domain2n1); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 3, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 3, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1921,7 +1915,7 @@ namespace ippl { if (ldom_g.touches(domain2n1)) { intersection = ldom_g.intersect(domain2n1); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 4, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 4, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1941,7 +1935,7 @@ namespace ippl { if (ldom_g.touches(domain2n1)) { intersection = ldom_g.intersect(domain2n1); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 5, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 5, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1961,7 +1955,7 @@ namespace ippl { if (ldom_g.touches(domain2n1)) { intersection = ldom_g.intersect(domain2n1); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 6, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 6, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1983,7 +1977,7 @@ namespace ippl { if (ldom_g.touches(domain2n1)) { intersection = ldom_g.intersect(domain2n1); - solver_send(mpi::tag::VICO_SEND, mpi::tag::VICO_SOLVER, 7, i, intersection, + solver_send(mpi::tag::VICO_SOLVER, 7, i, intersection, ldom_g, nghost_g, view_g, fd_m, requests); } } @@ -1997,7 +1991,7 @@ namespace ippl { if (lDomains2n1[i].touches(intersection)) { intersection = intersection.intersect(lDomains2n1[i]); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 0, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 0, i, intersection, ldom, nghost, view, fd_m); } } @@ -2020,7 +2014,7 @@ namespace ippl { intersection = intersection.intersect(domain2n1); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 1, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 1, i, intersection, ldom, nghost, view, fd_m, true, false, false); } } @@ -2043,7 +2037,7 @@ namespace ippl { intersection = intersection.intersect(domain2n1); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 2, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 2, i, intersection, ldom, nghost, view, fd_m, false, true, false); } } @@ -2066,7 +2060,7 @@ namespace ippl { intersection = intersection.intersect(domain2n1); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 3, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 3, i, intersection, ldom, nghost, view, fd_m, false, false, true); } } @@ -2093,7 +2087,7 @@ namespace ippl { intersection = intersection.intersect(domain2n1); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 4, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 4, i, intersection, ldom, nghost, view, fd_m, true, true, false); } } @@ -2120,7 +2114,7 @@ namespace ippl { intersection = intersection.intersect(domain2n1); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 5, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 5, i, intersection, ldom, nghost, view, fd_m, false, true, true); } } @@ -2147,7 +2141,7 @@ namespace ippl { intersection = intersection.intersect(domain2n1); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 6, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 6, i, intersection, ldom, nghost, view, fd_m, true, false, true); } } @@ -2178,7 +2172,7 @@ namespace ippl { intersection = intersection.intersect(domain2n1); - solver_recv(mpi::tag::VICO_RECV, mpi::tag::VICO_SOLVER, 7, i, intersection, + solver_recv(mpi::tag::VICO_SOLVER, 7, i, intersection, ldom, nghost, view, fd_m, true, true, true); } } From a6ff99c560d307ee30a655cee5c6e18a8623534a Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 4 Nov 2024 22:51:54 +0100 Subject: [PATCH 16/21] made file name variable and made print function work with the Inform object --- alpine/LandauDamping.cpp | 1 - src/Communicate/Communicator.h | 4 ++-- src/Communicate/CommunicatorLogging.cpp | 20 ++++++++++++-------- test/field/TestHalo.cpp | 2 +- test/particle/TestScatter.cpp | 1 - 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/alpine/LandauDamping.cpp b/alpine/LandauDamping.cpp index 9fc7af057..5322afd50 100644 --- a/alpine/LandauDamping.cpp +++ b/alpine/LandauDamping.cpp @@ -81,7 +81,6 @@ int main(int argc, char* argv[]) { IpplTimings::stopTimer(mainTimer); IpplTimings::print(); IpplTimings::print(std::string("timing.dat")); - ippl::Comm->printLogs(); } ippl::finalize(); diff --git a/src/Communicate/Communicator.h b/src/Communicate/Communicator.h index ec9de8d8d..24cf0c0e3 100644 --- a/src/Communicate/Communicator.h +++ b/src/Communicate/Communicator.h @@ -194,13 +194,13 @@ namespace ippl { MPI_Irecv(ar.getBuffer(), msize, MPI_BYTE, src, tag, *comm_m, &request); } - void printLogs(); + void printLogs(const std::string& filename); private: std::vector gatherLocalLogs(); void sendLogsToRank0(const std::vector& localLogs); std::vector gatherLogsFromAllRanks(const std::vector& localLogs); - void writeLogsToFile(const std::vector& allLogs); + void writeLogsToFile(const std::vector& allLogs, const std::string& filename); buffer_handler_type buffer_handlers_m; diff --git a/src/Communicate/CommunicatorLogging.cpp b/src/Communicate/CommunicatorLogging.cpp index 99668ae84..62237459a 100644 --- a/src/Communicate/CommunicatorLogging.cpp +++ b/src/Communicate/CommunicatorLogging.cpp @@ -1,13 +1,14 @@ #include "Communicate/Communicator.h" #include "Communicate/LogEntry.h" #include "Communicate/CommunicatorLogging.hpp" +#include "Utility/Inform.h" #include #include namespace ippl { namespace mpi { - void Communicator::printLogs() { + void Communicator::printLogs(const std::string& filename) { std::vector localLogs = gatherLocalLogs(); std::vector allLogs; @@ -18,7 +19,7 @@ namespace ippl { } if (rank() == 0) { - writeLogsToFile(allLogs); + writeLogsToFile(allLogs, filename); } } @@ -89,11 +90,14 @@ namespace ippl { return logs; } - void Communicator::writeLogsToFile(const std::vector& allLogs) { - std::ofstream logFile("log_entries.csv"); - - logFile << "Timestamp,Method,Rank,MemorySpace,AllocatedSize,FreeSize,Parameters\n"; + void Communicator::writeLogsToFile(const std::vector& allLogs, const std::string& filename) { + Inform logFile(0, filename.c_str(), Inform::OVERWRITE, 0); + logFile.setOutputLevel(1); + logFile << "Timestamp,Method,Rank,MemorySpace,AllocatedSize,FreeSize,Parameters" << endl; + std::cout << "hello world" << std::endl; + std::cout << allLogs.size() << std::endl; + for (const auto& log : allLogs) { auto timestamp = std::chrono::duration_cast( log.timestamp.time_since_epoch()) @@ -111,10 +115,10 @@ namespace ippl { logFile << key << ": " << value; first = false; } - logFile << "\"\n"; + logFile << "\"" << endl; } - logFile.close(); + logFile.flush(); } } // namespace mpi diff --git a/test/field/TestHalo.cpp b/test/field/TestHalo.cpp index b68db6abd..f7f651d3f 100644 --- a/test/field/TestHalo.cpp +++ b/test/field/TestHalo.cpp @@ -145,7 +145,7 @@ int main(int argc, char* argv[]) { IpplTimings::print(); IpplTimings::print(std::string("timing.dat")); - layout.comm.printLogs(); + layout.comm.printLogs("testfile.csv"); } ippl::finalize(); diff --git a/test/particle/TestScatter.cpp b/test/particle/TestScatter.cpp index d7500352b..d92f6a9b1 100644 --- a/test/particle/TestScatter.cpp +++ b/test/particle/TestScatter.cpp @@ -102,7 +102,6 @@ int main(int argc, char* argv[]) { } catch (const std::exception& e) { std::cout << e.what() << std::endl; } - ippl::Comm->printLogs(); } ippl::finalize(); From 8bfbe92f7d26af5aa5bacb97d41fbeb200ed5d60 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 4 Nov 2024 23:18:19 +0100 Subject: [PATCH 17/21] added hpp file for implementation of bufferhandler and added documentation of bufferhandler --- src/Communicate/BufferHandler.h | 188 ++++++++++++------------------ src/Communicate/BufferHandler.hpp | 142 ++++++++++++++++++++++ 2 files changed, 214 insertions(+), 116 deletions(-) create mode 100644 src/Communicate/BufferHandler.hpp diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h index 33b4f96a9..f2bcd9659 100644 --- a/src/Communicate/BufferHandler.h +++ b/src/Communicate/BufferHandler.h @@ -2,7 +2,7 @@ #define IPPL_BUFFER_HANDLER_H #include - +#include #include "Communicate/Archive.h" template @@ -23,6 +23,16 @@ class IBufferHandler { virtual size_type getFreeSize() const = 0; }; +/** + * @class BufferHandler + * @brief Concrete implementation of IBufferHandler for managing memory buffers. + * + * This class implements the IBufferHandler interface, providing concrete behavior for + * buffer allocation, freeing, and memory management. It maintains a pool of allocated + * and free buffers to optimize memory usage. + * + * @tparam MemorySpace The memory space type for the buffer (e.g., `Kokkos::HostSpace`). + */ template class BufferHandler : public IBufferHandler { public: @@ -30,128 +40,74 @@ class BufferHandler : public IBufferHandler { using buffer_type = std::shared_ptr; using size_type = ippl::detail::size_type; - ~BufferHandler() {} - - buffer_type getBuffer(size_type size, double overallocation) override { - size_type requiredSize = static_cast(size * overallocation); - - auto freeBuffer = findFreeBuffer(requiredSize); - - if (freeBuffer != nullptr) { - return allocateFromFreeBuffer(freeBuffer); - } - - if (!free_buffers.empty()) { - return reallocateLargestFreeBuffer(requiredSize); - } - - return allocateNewBuffer(requiredSize); - } - - void freeBuffer(buffer_type buffer) override { - if (isBufferUsed(buffer)) { - releaseUsedBuffer(buffer); - } - } - - void freeAllBuffers() override { - free_buffers.insert(used_buffers.begin(), used_buffers.end()); - used_buffers.clear(); - - freeSize += allocatedSize; - allocatedSize = 0; - } - - void deleteAllBuffers() override { - freeSize = 0; - allocatedSize = 0; - - used_buffers.clear(); - free_buffers.clear(); - } - - size_type getAllocatedSize() const override { return allocatedSize; } - - size_type getFreeSize() const override { return freeSize; } + ~BufferHandler() override; + + /** + * @brief Retrieves a buffer of the specified size, or creates a new one if needed. + * + * This function first searches for a free buffer of the requested size or larger. + * If none is found, it allocates a new buffer or reallocates an existing one. + * + * @param size The required size of the buffer. + * @param overallocation A multiplier to determine additional buffer space. + * @return A shared pointer to the allocated buffer. + */ + buffer_type getBuffer(size_type size, double overallocation) override; + + /** + * @brief Frees a specific buffer, returning it to the free buffer pool without deallocating the actual memory region. + * + * @param buffer The buffer to free. + */ + void freeBuffer(buffer_type buffer) override; + + /** + * @brief Frees all allocated buffers and adds them to the free pool without deallocating the actual memory region. + */ + void freeAllBuffers() override; + + /** + * @brief Deletes all buffers, thereby freeing the memory. + */ + void deleteAllBuffers() override; + + /** + * @brief Retrieves the total allocated size, i.e. the size of allocated memory currently in use. + * + * @return The total size of allocated buffers in use. + */ + size_type getAllocatedSize() const override; + + /** + * @brief Retrieves the total size of free buffers, i.e. size of allocated memory currently not in use. + * + * @return The total size of free buffers. + */ + size_type getFreeSize() const override; private: using buffer_comparator_type = bool (*)(const buffer_type&, const buffer_type&); using buffer_set_type = std::set; - static bool bufferSizeComparator(const buffer_type& lhs, const buffer_type& rhs) { - // Compare by size first, then by memory address to get total ordering - if (lhs->getBufferSize() != rhs->getBufferSize()) { - return lhs->getBufferSize() < rhs->getBufferSize(); - } - return lhs < rhs; - } - - bool isBufferUsed(buffer_type buffer) const { - return used_buffers.find(buffer) != used_buffers.end(); - } - - void releaseUsedBuffer(buffer_type buffer) { - auto it = used_buffers.find(buffer); - - allocatedSize -= buffer->getBufferSize(); - freeSize += buffer->getBufferSize(); - - used_buffers.erase(it); - free_buffers.insert(buffer); - } - - buffer_type findFreeBuffer(size_type requiredSize) { - auto it = findSmallestSufficientBuffer(requiredSize); - if (it != free_buffers.end()) { - return *it; - } - return nullptr; - } - - buffer_set_type::iterator findSmallestSufficientBuffer(size_type requiredSize) { - return std::find_if(free_buffers.begin(), free_buffers.end(), - [requiredSize](const buffer_type& buffer) { - return buffer->getBufferSize() >= requiredSize; - }); - } - - buffer_type allocateFromFreeBuffer(buffer_type buffer) { - freeSize -= buffer->getBufferSize(); - allocatedSize += buffer->getBufferSize(); - - free_buffers.erase(buffer); - used_buffers.insert(buffer); - return buffer; - } - - buffer_type reallocateLargestFreeBuffer(size_type requiredSize) { - auto largest_it = std::prev(free_buffers.end()); - buffer_type buffer = *largest_it; - - freeSize -= buffer->getBufferSize(); - allocatedSize += requiredSize; - - free_buffers.erase(buffer); - buffer->reallocBuffer(requiredSize); - - used_buffers.insert(buffer); - return buffer; - } - - buffer_type allocateNewBuffer(size_type requiredSize) { - buffer_type newBuffer = std::make_shared(requiredSize); - - allocatedSize += newBuffer->getBufferSize(); - used_buffers.insert(newBuffer); - return newBuffer; - } - - size_type allocatedSize; - size_type freeSize; + static bool bufferSizeComparator(const buffer_type& lhs, const buffer_type& rhs); + + bool isBufferUsed(buffer_type buffer) const; + void releaseUsedBuffer(buffer_type buffer); + buffer_type findFreeBuffer(size_type requiredSize); + buffer_set_type::iterator findSmallestSufficientBuffer(size_type requiredSize); + buffer_type allocateFromFreeBuffer(buffer_type buffer); + buffer_type reallocateLargestFreeBuffer(size_type requiredSize); + buffer_type allocateNewBuffer(size_type requiredSize); + + size_type allocatedSize; ///< Total size of all allocated buffers + size_type freeSize; ///< Total size of all free buffers protected: - buffer_set_type used_buffers{&BufferHandler::bufferSizeComparator}; - buffer_set_type free_buffers{&BufferHandler::bufferSizeComparator}; + buffer_set_type used_buffers{&BufferHandler::bufferSizeComparator}; ///< Set of used buffers + buffer_set_type free_buffers{&BufferHandler::bufferSizeComparator}; ///< Set of free buffers }; +#include "BufferHandler.hpp" + #endif + diff --git a/src/Communicate/BufferHandler.hpp b/src/Communicate/BufferHandler.hpp new file mode 100644 index 000000000..3363887d3 --- /dev/null +++ b/src/Communicate/BufferHandler.hpp @@ -0,0 +1,142 @@ +#ifndef IPPL_BUFFER_HANDLER_HPP +#define IPPL_BUFFER_HANDLER_HPP + + +template +BufferHandler::~BufferHandler() {} + +template +typename BufferHandler::buffer_type +BufferHandler::getBuffer(size_type size, double overallocation) { + size_type requiredSize = static_cast(size * overallocation); + + auto freeBuffer = findFreeBuffer(requiredSize); + if (freeBuffer != nullptr) { + return allocateFromFreeBuffer(freeBuffer); + } + + if (!free_buffers.empty()) { + return reallocateLargestFreeBuffer(requiredSize); + } + + return allocateNewBuffer(requiredSize); +} + +template +void BufferHandler::freeBuffer(buffer_type buffer) { + if (isBufferUsed(buffer)) { + releaseUsedBuffer(buffer); + } +} + +template +void BufferHandler::freeAllBuffers() { + free_buffers.insert(used_buffers.begin(), used_buffers.end()); + used_buffers.clear(); + + freeSize += allocatedSize; + allocatedSize = 0; +} + +template +void BufferHandler::deleteAllBuffers() { + freeSize = 0; + allocatedSize = 0; + + used_buffers.clear(); + free_buffers.clear(); +} + +template +typename BufferHandler::size_type +BufferHandler::getAllocatedSize() const { + return allocatedSize; +} + +template +typename BufferHandler::size_type +BufferHandler::getFreeSize() const { + return freeSize; +} + +template +bool BufferHandler::bufferSizeComparator(const buffer_type& lhs, const buffer_type& rhs) { + if (lhs->getBufferSize() != rhs->getBufferSize()) { + return lhs->getBufferSize() < rhs->getBufferSize(); + } + return lhs < rhs; +} + +template +bool BufferHandler::isBufferUsed(buffer_type buffer) const { + return used_buffers.find(buffer) != used_buffers.end(); +} + +template +void BufferHandler::releaseUsedBuffer(buffer_type buffer) { + auto it = used_buffers.find(buffer); + + allocatedSize -= buffer->getBufferSize(); + freeSize += buffer->getBufferSize(); + + used_buffers.erase(it); + free_buffers.insert(buffer); +} + +template +typename BufferHandler::buffer_type +BufferHandler::findFreeBuffer(size_type requiredSize) { + auto it = findSmallestSufficientBuffer(requiredSize); + if (it != free_buffers.end()) { + return *it; + } + return nullptr; +} + +template +typename BufferHandler::buffer_set_type::iterator +BufferHandler::findSmallestSufficientBuffer(size_type requiredSize) { + return std::find_if(free_buffers.begin(), free_buffers.end(), + [requiredSize](const buffer_type& buffer) { + return buffer->getBufferSize() >= requiredSize; + }); +} + +template +typename BufferHandler::buffer_type +BufferHandler::allocateFromFreeBuffer(buffer_type buffer) { + freeSize -= buffer->getBufferSize(); + allocatedSize += buffer->getBufferSize(); + + free_buffers.erase(buffer); + used_buffers.insert(buffer); + return buffer; +} + +template +typename BufferHandler::buffer_type +BufferHandler::reallocateLargestFreeBuffer(size_type requiredSize) { + auto largest_it = std::prev(free_buffers.end()); + buffer_type buffer = *largest_it; + + freeSize -= buffer->getBufferSize(); + allocatedSize += requiredSize; + + free_buffers.erase(buffer); + buffer->reallocBuffer(requiredSize); + + used_buffers.insert(buffer); + return buffer; +} + +template +typename BufferHandler::buffer_type +BufferHandler::allocateNewBuffer(size_type requiredSize) { + buffer_type newBuffer = std::make_shared(requiredSize); + + allocatedSize += newBuffer->getBufferSize(); + used_buffers.insert(newBuffer); + return newBuffer; +} + +#endif From 6f8ec5a35df80022a6b36fa4d3e413090e125410 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 4 Nov 2024 23:29:12 +0100 Subject: [PATCH 18/21] updated documentation for interface of buffer handler. --- src/Communicate/BufferHandler.h | 78 +++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 9 deletions(-) diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h index f2bcd9659..bcebc94fb 100644 --- a/src/Communicate/BufferHandler.h +++ b/src/Communicate/BufferHandler.h @@ -5,6 +5,15 @@ #include #include "Communicate/Archive.h" +/** + * @brief Interface for memory buffer handling. + * + * Defines methods for acquiring, freeing, and managing memory buffers. + * Implementations are responsible for managing buffers efficiently, + * ensuring that allocated buffers are reused where possible. + * + * @tparam MemorySpace The memory space type used for buffer allocation. + */ template class IBufferHandler { public: @@ -14,13 +23,59 @@ class IBufferHandler { virtual ~IBufferHandler() {} + /** + * @brief Requests a memory buffer of a specified size. + * + * Provides a buffer of at least the specified size, with the option + * to allocate additional space based on an overallocation multiplier. + * This function attempts to reuse available buffers if possible. + * + * @param size The required size of the buffer, in bytes. + * @param overallocation A multiplier to allocate extra space, which may help + * avoid frequent reallocation in some use cases. + * @return A shared pointer to the allocated buffer. + */ virtual buffer_type getBuffer(size_type size, double overallocation) = 0; + + /** + * @brief Frees a specified buffer. + * + * Moves the specified buffer to a free state, making it available + * for reuse in future buffer requests. + * + * @param buffer The buffer to be freed. + */ virtual void freeBuffer(buffer_type buffer) = 0; + + /** + * @brief Frees all currently used buffers. + * + * Transfers all used buffers to the free state, making them available + * for reuse. This does not deallocate memory but resets buffer usage. + */ virtual void freeAllBuffers() = 0; + + /** + * @brief Deletes all buffers. + * + * Releases all allocated memory buffers, both used and free. + * After this call, no buffers are available until new allocations. + */ virtual void deleteAllBuffers() = 0; + /** + * @brief Gets the size of all allocated buffers. + * + * @return Total size of allocated buffers in bytes. + */ virtual size_type getAllocatedSize() const = 0; - virtual size_type getFreeSize() const = 0; + + /** + * @brief Gets the size of all free buffers. + * + * @return Total size of free buffers in bytes. + */ + virtual size_type getFreeSize() const = 0; }; /** @@ -43,21 +98,26 @@ class BufferHandler : public IBufferHandler { ~BufferHandler() override; /** - * @brief Retrieves a buffer of the specified size, or creates a new one if needed. + * @brief Acquires a buffer of at least the specified size. * - * This function first searches for a free buffer of the requested size or larger. - * If none is found, it allocates a new buffer or reallocates an existing one. - * - * @param size The required size of the buffer. - * @param overallocation A multiplier to determine additional buffer space. + * Requests a memory buffer of the specified size, with the option + * to request a buffer larger than the base size by an overallocation + * multiplier. Implementations should attempt to reuse existing + * buffers if possible. + * + * @param size The required buffer size. + * @param overallocation A multiplier to allocate additional buffer space. * @return A shared pointer to the allocated buffer. */ buffer_type getBuffer(size_type size, double overallocation) override; /** - * @brief Frees a specific buffer, returning it to the free buffer pool without deallocating the actual memory region. + * @brief Frees a specified buffer. + * + * Moves the specified buffer to a free state, making it available + * for reuse in future buffer requests. * - * @param buffer The buffer to free. + * @param buffer The buffer to be freed. */ void freeBuffer(buffer_type buffer) override; From 6689800852b90795286bbb6f9a495d4346c1d06d Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Mon, 4 Nov 2024 23:44:26 +0100 Subject: [PATCH 19/21] separated loggingbufferhandler into h and hpp file and added documentation --- src/Communicate/BufferHandler.h | 2 +- src/Communicate/LoggingBufferHandler.h | 136 +++++++++++++++-------- src/Communicate/LoggingBufferHandler.hpp | 73 ++++++++++++ unit_tests/Communicate/BufferHandler.cpp | 7 -- 4 files changed, 163 insertions(+), 55 deletions(-) create mode 100644 src/Communicate/LoggingBufferHandler.hpp diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h index bcebc94fb..444cd81ef 100644 --- a/src/Communicate/BufferHandler.h +++ b/src/Communicate/BufferHandler.h @@ -167,7 +167,7 @@ class BufferHandler : public IBufferHandler { buffer_set_type free_buffers{&BufferHandler::bufferSizeComparator}; ///< Set of free buffers }; -#include "BufferHandler.hpp" +#include "Communicate/BufferHandler.hpp" #endif diff --git a/src/Communicate/LoggingBufferHandler.h b/src/Communicate/LoggingBufferHandler.h index a10580bfd..96abde583 100644 --- a/src/Communicate/LoggingBufferHandler.h +++ b/src/Communicate/LoggingBufferHandler.h @@ -9,70 +9,112 @@ #include "Communicate/BufferHandler.h" #include "Communicate/LogEntry.h" +/** + * @class LoggingBufferHandler + * @brief Decorator class for buffer management that adds logging capabilities to buffer operations. + * + * `LoggingBufferHandler` extends the basic functionality of `IBufferHandler` by recording + * detailed logs of buffer operations, such as allocation, deallocation, and resizing. + * This allows tracking of memory usage patterns and provides a record of buffer activity. + * + * @tparam MemorySpace The memory space in which buffers are managed (e.g., HostSpace, CudaSpace). + * + * This class is a decorator for `BufferHandler` and does not modify buffer management logic. + * Instead, it adds logging for monitoring purposes. + */ template class LoggingBufferHandler : public IBufferHandler { public: using buffer_type = typename IBufferHandler::buffer_type; using size_type = typename IBufferHandler::size_type; - LoggingBufferHandler(std::shared_ptr> handler, int rank) - : handler_(std::move(handler)), rank_(rank) {} + /** + * @brief Constructs a LoggingBufferHandler with an existing buffer handler. + * @param handler A shared pointer to an `IBufferHandler` instance used for buffer operations. + * @param rank The MPI rank for logging purposes, used to identify the source of logs. + */ + LoggingBufferHandler(std::shared_ptr> handler, int rank); - LoggingBufferHandler() { - handler_ = std::make_shared>(); - MPI_Comm_rank(MPI_COMM_WORLD, &rank_); - } + /** + * @brief Default constructor, creates an internal `BufferHandler` for managing buffers. + * This constructor also initializes the rank by calling `MPI_Comm_rank`. + */ + LoggingBufferHandler(); - buffer_type getBuffer(size_type size, double overallocation) override { - auto buffer = handler_->getBuffer(size, overallocation); - logMethod("getBuffer", {{"size", std::to_string(size)}, {"overallocation", std::to_string(overallocation)}}); - return buffer; - } + /** + * @brief Allocates or retrieves a buffer and logs the action. + * + * Overrides `IBufferHandler::getBuffer`, providing the same buffer allocation behavior + * while recording an entry in the log with the operation details. + * + * @param size Requested size of the buffer. + * @param overallocation Optional multiplier to allocate extra buffer space. + * @return A buffer object. + */ + buffer_type getBuffer(size_type size, double overallocation) override; - void freeBuffer(buffer_type buffer) override { - handler_->freeBuffer(buffer); - logMethod("freeBuffer", {}); - } + /** + * @brief Frees a buffer and logs the action. + * + * Overrides `IBufferHandler::freeBuffer`. Calls `BufferHandler::freeBuffer` and records the + * operation in the log. + * + * @param buffer The buffer to be freed. + */ + void freeBuffer(buffer_type buffer) override; - void freeAllBuffers() override { - handler_->freeAllBuffers(); - logMethod("freeAllBuffers", {}); - } + /** + * @brief Frees all buffers and logs the action. + * + * Overrides `IBufferHandler::freeAllBuffers`. Calls `BufferHandler::freeAllBuffers` and logs + * the operation. + */ + void freeAllBuffers() override; - void deleteAllBuffers() override { - handler_->deleteAllBuffers(); - logMethod("deleteAllBuffers", {}); - } + /** + * @brief Deletes all buffers and logs the action. + * + * Overrides `IBufferHandler::deleteAllBuffers`. Calls `BufferHandler::deleteAllBuffers` and + * logs the operation. + */ + void deleteAllBuffers() override; - size_type getAllocatedSize() const override { - return handler_->getAllocatedSize(); - } + /** + * @brief Retrieves the total size of allocated buffers. + * @return The size of allocated buffers. + */ + size_type getAllocatedSize() const override; - size_type getFreeSize() const override { - return handler_->getFreeSize(); - } + /** + * @brief Retrieves the total size of free buffers. + * @return The size of free buffers. + */ + size_type getFreeSize() const override; - const std::vector& getLogs() const { - std::cout << logEntries_.size() << std::endl; - return logEntries_; - } + /** + * @brief Retrieves the list of log entries. + * @return A constant reference to a vector containing log entries. + */ + const std::vector& getLogs() const; private: - std::shared_ptr> handler_; - std::vector logEntries_; - int rank_; + std::shared_ptr> handler_; ///< Internal handler for buffer management. + std::vector logEntries_; ///< Log entries for buffer operations. + int rank_; ///< MPI rank for identifying log sources. - void logMethod(const std::string& methodName, const std::map& parameters) { - logEntries_.push_back({ - methodName, - parameters, - handler_->getAllocatedSize(), - handler_->getFreeSize(), - MemorySpace::name(), - rank_, - std::chrono::high_resolution_clock::now() - }); - } + /** + * @brief Records a method call in the log with its parameters. + * + * This method creates a log entry with details of the buffer operation, + * including the method name, parameters, allocated size, free size, + * memory space, rank, and timestamp. + * + * @param methodName Name of the method being logged (e.g., "getBuffer"). + * @param parameters A map of parameter names and values for the operation. + */ + void logMethod(const std::string& methodName, const std::map& parameters); }; +#include "Communicate/LoggingBufferHandler.hpp" + #endif diff --git a/src/Communicate/LoggingBufferHandler.hpp b/src/Communicate/LoggingBufferHandler.hpp new file mode 100644 index 000000000..c66fde348 --- /dev/null +++ b/src/Communicate/LoggingBufferHandler.hpp @@ -0,0 +1,73 @@ +#ifndef IPPL_LOGGING_BUFFER_HANDLER_HPP +#define IPPL_LOGGING_BUFFER_HANDLER_HPP + +#include +#include + +template +LoggingBufferHandler::LoggingBufferHandler(std::shared_ptr> handler, int rank) + : handler_(std::move(handler)), rank_(rank) {} + +template +LoggingBufferHandler::LoggingBufferHandler() { + handler_ = std::make_shared>(); + MPI_Comm_rank(MPI_COMM_WORLD, &rank_); +} + +template +typename LoggingBufferHandler::buffer_type LoggingBufferHandler::getBuffer( + size_type size, double overallocation) { + auto buffer = handler_->getBuffer(size, overallocation); + logMethod("getBuffer", {{"size", std::to_string(size)}, {"overallocation", std::to_string(overallocation)}}); + return buffer; +} + +template +void LoggingBufferHandler::freeBuffer(buffer_type buffer) { + handler_->freeBuffer(buffer); + logMethod("freeBuffer", {}); +} + +template +void LoggingBufferHandler::freeAllBuffers() { + handler_->freeAllBuffers(); + logMethod("freeAllBuffers", {}); +} + +template +void LoggingBufferHandler::deleteAllBuffers() { + handler_->deleteAllBuffers(); + logMethod("deleteAllBuffers", {}); +} + +template +typename LoggingBufferHandler::size_type LoggingBufferHandler::getAllocatedSize() const { + return handler_->getAllocatedSize(); +} + +template +typename LoggingBufferHandler::size_type LoggingBufferHandler::getFreeSize() const { + return handler_->getFreeSize(); +} + +template +const std::vector& LoggingBufferHandler::getLogs() const { + std::cout << logEntries_.size() << std::endl; + return logEntries_; +} + +template +void LoggingBufferHandler::logMethod(const std::string& methodName, const std::map& parameters) { + logEntries_.push_back({ + methodName, + parameters, + handler_->getAllocatedSize(), + handler_->getFreeSize(), + MemorySpace::name(), + rank_, + std::chrono::high_resolution_clock::now() + }); +} + +#endif + diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp index 39ada92d8..3c9b3e4f0 100644 --- a/unit_tests/Communicate/BufferHandler.cpp +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -8,13 +8,6 @@ using MemorySpaces = ippl::detail::TypeForAllSpaces<::testing::Types>::memory_spaces_type; -template -void debugType() { - std::cout << __PRETTY_FUNCTION__ << '\n'; -} - - - template class TypedBufferHandlerTest : public ::testing::Test { protected: From 10ecd879d4bd07825629a0bd86fc85f23f3b1cdc Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Wed, 6 Nov 2024 16:05:45 +0100 Subject: [PATCH 20/21] incorporated suggested changes into PR --- src/Communicate/BufferHandler.h | 314 +++++++++--------- src/Communicate/BufferHandler.hpp | 257 +++++++------- src/Communicate/Buffers.hpp | 2 +- src/Communicate/CommunicatorLogging.cpp | 6 +- src/Communicate/LogEntry.cpp | 117 +++---- src/Communicate/LogEntry.h | 64 ++-- src/Communicate/LoggingBufferHandler.h | 199 +++++------ src/Communicate/LoggingBufferHandler.hpp | 116 +++---- test/field/TestHalo.cpp | 2 +- unit_tests/Communicate/BufferHandler.cpp | 18 +- unit_tests/Communicate/LogEntry.cpp | 48 +-- .../Communicate/LoggingBufferHandler.cpp | 16 +- 12 files changed, 597 insertions(+), 562 deletions(-) diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h index 444cd81ef..6c06cd29c 100644 --- a/src/Communicate/BufferHandler.h +++ b/src/Communicate/BufferHandler.h @@ -1,173 +1,181 @@ #ifndef IPPL_BUFFER_HANDLER_H #define IPPL_BUFFER_HANDLER_H -#include #include -#include "Communicate/Archive.h" - -/** - * @brief Interface for memory buffer handling. - * - * Defines methods for acquiring, freeing, and managing memory buffers. - * Implementations are responsible for managing buffers efficiently, - * ensuring that allocated buffers are reused where possible. - * - * @tparam MemorySpace The memory space type used for buffer allocation. - */ -template -class IBufferHandler { -public: - using archive_type = ippl::detail::Archive; - using buffer_type = std::shared_ptr; - using size_type = ippl::detail::size_type; - - virtual ~IBufferHandler() {} - - /** - * @brief Requests a memory buffer of a specified size. - * - * Provides a buffer of at least the specified size, with the option - * to allocate additional space based on an overallocation multiplier. - * This function attempts to reuse available buffers if possible. - * - * @param size The required size of the buffer, in bytes. - * @param overallocation A multiplier to allocate extra space, which may help - * avoid frequent reallocation in some use cases. - * @return A shared pointer to the allocated buffer. - */ - virtual buffer_type getBuffer(size_type size, double overallocation) = 0; - - /** - * @brief Frees a specified buffer. - * - * Moves the specified buffer to a free state, making it available - * for reuse in future buffer requests. - * - * @param buffer The buffer to be freed. - */ - virtual void freeBuffer(buffer_type buffer) = 0; - - /** - * @brief Frees all currently used buffers. - * - * Transfers all used buffers to the free state, making them available - * for reuse. This does not deallocate memory but resets buffer usage. - */ - virtual void freeAllBuffers() = 0; - - /** - * @brief Deletes all buffers. - * - * Releases all allocated memory buffers, both used and free. - * After this call, no buffers are available until new allocations. - */ - virtual void deleteAllBuffers() = 0; +#include - /** - * @brief Gets the size of all allocated buffers. - * - * @return Total size of allocated buffers in bytes. - */ - virtual size_type getAllocatedSize() const = 0; +#include "Communicate/Archive.h" - /** - * @brief Gets the size of all free buffers. - * - * @return Total size of free buffers in bytes. - */ - virtual size_type getFreeSize() const = 0; -}; - -/** - * @class BufferHandler - * @brief Concrete implementation of IBufferHandler for managing memory buffers. - * - * This class implements the IBufferHandler interface, providing concrete behavior for - * buffer allocation, freeing, and memory management. It maintains a pool of allocated - * and free buffers to optimize memory usage. - * - * @tparam MemorySpace The memory space type for the buffer (e.g., `Kokkos::HostSpace`). - */ -template -class BufferHandler : public IBufferHandler { -public: - using archive_type = ippl::detail::Archive; - using buffer_type = std::shared_ptr; - using size_type = ippl::detail::size_type; - - ~BufferHandler() override; +namespace ippl { /** - * @brief Acquires a buffer of at least the specified size. + * @brief Interface for memory buffer handling. * - * Requests a memory buffer of the specified size, with the option - * to request a buffer larger than the base size by an overallocation - * multiplier. Implementations should attempt to reuse existing - * buffers if possible. + * Defines methods for acquiring, freeing, and managing memory buffers. + * Implementations are responsible for managing buffers efficiently, + * ensuring that allocated buffers are reused where possible. * - * @param size The required buffer size. - * @param overallocation A multiplier to allocate additional buffer space. - * @return A shared pointer to the allocated buffer. + * @tparam MemorySpace The memory space type used for buffer allocation. */ - buffer_type getBuffer(size_type size, double overallocation) override; + template + class IBufferHandler { + public: + using archive_type = ippl::detail::Archive; + using buffer_type = std::shared_ptr; + using size_type = ippl::detail::size_type; + + virtual ~IBufferHandler() {} + + /** + * @brief Requests a memory buffer of a specified size. + * + * Provides a buffer of at least the specified size, with the option + * to allocate additional space based on an overallocation multiplier. + * This function attempts to reuse available buffers if possible. + * + * @param size The required size of the buffer, in bytes. + * @param overallocation A multiplier to allocate extra space, which may help + * avoid frequent reallocation in some use cases. + * @return A shared pointer to the allocated buffer. + */ + virtual buffer_type getBuffer(size_type size, double overallocation) = 0; + + /** + * @brief Frees a specified buffer. + * + * Moves the specified buffer to a free state, making it available + * for reuse in future buffer requests. + * + * @param buffer The buffer to be freed. + */ + virtual void freeBuffer(buffer_type buffer) = 0; + + /** + * @brief Frees all currently used buffers. + * + * Transfers all used buffers to the free state, making them available + * for reuse. This does not deallocate memory but resets buffer usage. + */ + virtual void freeAllBuffers() = 0; + + /** + * @brief Deletes all buffers. + * + * Releases all allocated memory buffers, both used and free. + * After this call, no buffers are available until new allocations. + */ + virtual void deleteAllBuffers() = 0; + + /** + * @brief Gets the size of all allocated buffers. + * + * @return Total size of allocated buffers in bytes. + */ + virtual size_type getUsedSize() const = 0; + + /** + * @brief Gets the size of all free buffers. + * + * @return Total size of free buffers in bytes. + */ + virtual size_type getFreeSize() const = 0; + }; /** - * @brief Frees a specified buffer. + * @class BufferHandler + * @brief Concrete implementation of IBufferHandler for managing memory buffers. * - * Moves the specified buffer to a free state, making it available - * for reuse in future buffer requests. + * This class implements the IBufferHandler interface, providing concrete behavior for + * buffer allocation, freeing, and memory management. It maintains a pool of allocated + * and free buffers to optimize memory usage. * - * @param buffer The buffer to be freed. - */ - void freeBuffer(buffer_type buffer) override; - - /** - * @brief Frees all allocated buffers and adds them to the free pool without deallocating the actual memory region. - */ - void freeAllBuffers() override; - - /** - * @brief Deletes all buffers, thereby freeing the memory. + * @tparam MemorySpace The memory space type for the buffer (e.g., `Kokkos::HostSpace`). */ - void deleteAllBuffers() override; - - /** - * @brief Retrieves the total allocated size, i.e. the size of allocated memory currently in use. - * - * @return The total size of allocated buffers in use. - */ - size_type getAllocatedSize() const override; - - /** - * @brief Retrieves the total size of free buffers, i.e. size of allocated memory currently not in use. - * - * @return The total size of free buffers. - */ - size_type getFreeSize() const override; - -private: - using buffer_comparator_type = bool (*)(const buffer_type&, const buffer_type&); - using buffer_set_type = std::set; - - static bool bufferSizeComparator(const buffer_type& lhs, const buffer_type& rhs); - - bool isBufferUsed(buffer_type buffer) const; - void releaseUsedBuffer(buffer_type buffer); - buffer_type findFreeBuffer(size_type requiredSize); - buffer_set_type::iterator findSmallestSufficientBuffer(size_type requiredSize); - buffer_type allocateFromFreeBuffer(buffer_type buffer); - buffer_type reallocateLargestFreeBuffer(size_type requiredSize); - buffer_type allocateNewBuffer(size_type requiredSize); - - size_type allocatedSize; ///< Total size of all allocated buffers - size_type freeSize; ///< Total size of all free buffers - -protected: - buffer_set_type used_buffers{&BufferHandler::bufferSizeComparator}; ///< Set of used buffers - buffer_set_type free_buffers{&BufferHandler::bufferSizeComparator}; ///< Set of free buffers -}; + template + class BufferHandler : public IBufferHandler { + public: + using typename IBufferHandler::archive_type; + using typename IBufferHandler::buffer_type; + using typename IBufferHandler::size_type; + + ~BufferHandler() override; + + /** + * @brief Acquires a buffer of at least the specified size. + * + * Requests a memory buffer of the specified size, with the option + * to request a buffer larger than the base size by an overallocation + * multiplier. Implementations should attempt to reuse existing + * buffers if possible. + * + * @param size The required buffer size. + * @param overallocation A multiplier to allocate additional buffer space. + * @return A shared pointer to the allocated buffer. + */ + buffer_type getBuffer(size_type size, double overallocation) override; + + /** + * @brief Frees a specified buffer. + * + * Moves the specified buffer to a free state, making it available + * for reuse in future buffer requests. + * + * @param buffer The buffer to be freed. + */ + void freeBuffer(buffer_type buffer) override; + + /** + * @brief Frees all allocated buffers and adds them to the free pool without deallocating + * the actual memory region. + */ + void freeAllBuffers() override; + + /** + * @brief Deletes all buffers, thereby freeing the memory. + */ + void deleteAllBuffers() override; + + /** + * @brief Retrieves the total allocated size, i.e. the size of allocated memory currently in + * use. + * + * @return The total size of allocated buffers in use. + */ + size_type getUsedSize() const override; + + /** + * @brief Retrieves the total size of free buffers, i.e. size of allocated memory currently + * not in use. + * + * @return The total size of free buffers. + */ + size_type getFreeSize() const override; + + private: + using buffer_comparator_type = bool (*)(const buffer_type&, const buffer_type&); + using buffer_set_type = std::set; + + static bool bufferSizeComparator(const buffer_type& lhs, const buffer_type& rhs); + + bool isBufferUsed(buffer_type buffer) const; + void releaseUsedBuffer(buffer_type buffer); + buffer_type findFreeBuffer(size_type requiredSize); + buffer_set_type::iterator findSmallestSufficientBuffer(size_type requiredSize); + buffer_type getFreeBuffer(buffer_type buffer); + buffer_type reallocateLargestFreeBuffer(size_type requiredSize); + buffer_type allocateNewBuffer(size_type requiredSize); + + size_type usedSize_m; ///< Total size of all allocated buffers + size_type freeSize_m; ///< Total size of all free buffers + + protected: + buffer_set_type used_buffers{ + &BufferHandler::bufferSizeComparator}; ///< Set of used buffers + buffer_set_type free_buffers{ + &BufferHandler::bufferSizeComparator}; ///< Set of free buffers + }; +} // namespace ippl #include "Communicate/BufferHandler.hpp" #endif - diff --git a/src/Communicate/BufferHandler.hpp b/src/Communicate/BufferHandler.hpp index 3363887d3..552c8d7e3 100644 --- a/src/Communicate/BufferHandler.hpp +++ b/src/Communicate/BufferHandler.hpp @@ -1,142 +1,147 @@ #ifndef IPPL_BUFFER_HANDLER_HPP #define IPPL_BUFFER_HANDLER_HPP +namespace ippl { -template -BufferHandler::~BufferHandler() {} + template + BufferHandler::~BufferHandler() {} -template -typename BufferHandler::buffer_type -BufferHandler::getBuffer(size_type size, double overallocation) { - size_type requiredSize = static_cast(size * overallocation); + template + typename BufferHandler::buffer_type BufferHandler::getBuffer( + size_type size, double overallocation) { + size_type requiredSize = static_cast(size * overallocation); - auto freeBuffer = findFreeBuffer(requiredSize); - if (freeBuffer != nullptr) { - return allocateFromFreeBuffer(freeBuffer); + auto freeBuffer = findFreeBuffer(requiredSize); + if (freeBuffer != nullptr) { + return getFreeBuffer(freeBuffer); + } + + if (!free_buffers.empty()) { + return reallocateLargestFreeBuffer(requiredSize); + } + + return allocateNewBuffer(requiredSize); + } + + template + void BufferHandler::freeBuffer(buffer_type buffer) { + if (isBufferUsed(buffer)) { + releaseUsedBuffer(buffer); + } + } + + template + void BufferHandler::freeAllBuffers() { + free_buffers.insert(used_buffers.begin(), used_buffers.end()); + used_buffers.clear(); + + freeSize_m += usedSize_m; + usedSize_m = 0; + } + + template + void BufferHandler::deleteAllBuffers() { + freeSize_m = 0; + usedSize_m = 0; + + used_buffers.clear(); + free_buffers.clear(); + } + + template + typename BufferHandler::size_type BufferHandler::getUsedSize() + const { + return usedSize_m; + } + + template + typename BufferHandler::size_type BufferHandler::getFreeSize() const { + return freeSize_m; + } + + template + bool BufferHandler::bufferSizeComparator(const buffer_type& lhs, + const buffer_type& rhs) { + if (lhs->getBufferSize() != rhs->getBufferSize()) { + return lhs->getBufferSize() < rhs->getBufferSize(); + } + + // Use memory address as a tie-breaker to enforce total ordering of buffers. + return lhs < rhs; + } + + template + bool BufferHandler::isBufferUsed(buffer_type buffer) const { + return used_buffers.find(buffer) != used_buffers.end(); + } + + template + void BufferHandler::releaseUsedBuffer(buffer_type buffer) { + auto it = used_buffers.find(buffer); + + usedSize_m -= buffer->getBufferSize(); + freeSize_m += buffer->getBufferSize(); + + used_buffers.erase(it); + free_buffers.insert(buffer); + } + + template + typename BufferHandler::buffer_type BufferHandler::findFreeBuffer( + size_type requiredSize) { + auto it = findSmallestSufficientBuffer(requiredSize); + if (it != free_buffers.end()) { + return *it; + } + return nullptr; } - if (!free_buffers.empty()) { - return reallocateLargestFreeBuffer(requiredSize); + template + typename BufferHandler::buffer_set_type::iterator + BufferHandler::findSmallestSufficientBuffer(size_type requiredSize) { + return std::find_if(free_buffers.begin(), free_buffers.end(), + [requiredSize](const buffer_type& buffer) { + return buffer->getBufferSize() >= requiredSize; + }); } - return allocateNewBuffer(requiredSize); -} + template + typename BufferHandler::buffer_type + BufferHandler::getFreeBuffer(buffer_type buffer) { + freeSize_m -= buffer->getBufferSize(); + usedSize_m += buffer->getBufferSize(); -template -void BufferHandler::freeBuffer(buffer_type buffer) { - if (isBufferUsed(buffer)) { - releaseUsedBuffer(buffer); + free_buffers.erase(buffer); + used_buffers.insert(buffer); + return buffer; } -} - -template -void BufferHandler::freeAllBuffers() { - free_buffers.insert(used_buffers.begin(), used_buffers.end()); - used_buffers.clear(); - - freeSize += allocatedSize; - allocatedSize = 0; -} - -template -void BufferHandler::deleteAllBuffers() { - freeSize = 0; - allocatedSize = 0; - - used_buffers.clear(); - free_buffers.clear(); -} - -template -typename BufferHandler::size_type -BufferHandler::getAllocatedSize() const { - return allocatedSize; -} - -template -typename BufferHandler::size_type -BufferHandler::getFreeSize() const { - return freeSize; -} - -template -bool BufferHandler::bufferSizeComparator(const buffer_type& lhs, const buffer_type& rhs) { - if (lhs->getBufferSize() != rhs->getBufferSize()) { - return lhs->getBufferSize() < rhs->getBufferSize(); + + template + typename BufferHandler::buffer_type + BufferHandler::reallocateLargestFreeBuffer(size_type requiredSize) { + auto largest_it = std::prev(free_buffers.end()); + buffer_type buffer = *largest_it; + + freeSize_m -= buffer->getBufferSize(); + usedSize_m += requiredSize; + + free_buffers.erase(buffer); + buffer->reallocBuffer(requiredSize); + + used_buffers.insert(buffer); + return buffer; } - return lhs < rhs; -} - -template -bool BufferHandler::isBufferUsed(buffer_type buffer) const { - return used_buffers.find(buffer) != used_buffers.end(); -} - -template -void BufferHandler::releaseUsedBuffer(buffer_type buffer) { - auto it = used_buffers.find(buffer); - - allocatedSize -= buffer->getBufferSize(); - freeSize += buffer->getBufferSize(); - - used_buffers.erase(it); - free_buffers.insert(buffer); -} - -template -typename BufferHandler::buffer_type -BufferHandler::findFreeBuffer(size_type requiredSize) { - auto it = findSmallestSufficientBuffer(requiredSize); - if (it != free_buffers.end()) { - return *it; + + template + typename BufferHandler::buffer_type BufferHandler::allocateNewBuffer( + size_type requiredSize) { + buffer_type newBuffer = std::make_shared(requiredSize); + + usedSize_m += newBuffer->getBufferSize(); + used_buffers.insert(newBuffer); + return newBuffer; } - return nullptr; -} - -template -typename BufferHandler::buffer_set_type::iterator -BufferHandler::findSmallestSufficientBuffer(size_type requiredSize) { - return std::find_if(free_buffers.begin(), free_buffers.end(), - [requiredSize](const buffer_type& buffer) { - return buffer->getBufferSize() >= requiredSize; - }); -} - -template -typename BufferHandler::buffer_type -BufferHandler::allocateFromFreeBuffer(buffer_type buffer) { - freeSize -= buffer->getBufferSize(); - allocatedSize += buffer->getBufferSize(); - - free_buffers.erase(buffer); - used_buffers.insert(buffer); - return buffer; -} - -template -typename BufferHandler::buffer_type -BufferHandler::reallocateLargestFreeBuffer(size_type requiredSize) { - auto largest_it = std::prev(free_buffers.end()); - buffer_type buffer = *largest_it; - - freeSize -= buffer->getBufferSize(); - allocatedSize += requiredSize; - - free_buffers.erase(buffer); - buffer->reallocBuffer(requiredSize); - - used_buffers.insert(buffer); - return buffer; -} - -template -typename BufferHandler::buffer_type -BufferHandler::allocateNewBuffer(size_type requiredSize) { - buffer_type newBuffer = std::make_shared(requiredSize); - - allocatedSize += newBuffer->getBufferSize(); - used_buffers.insert(newBuffer); - return newBuffer; -} + +} // namespace ippl #endif diff --git a/src/Communicate/Buffers.hpp b/src/Communicate/Buffers.hpp index 7fd72860d..bcc2aba77 100644 --- a/src/Communicate/Buffers.hpp +++ b/src/Communicate/Buffers.hpp @@ -36,7 +36,7 @@ namespace ippl { void Communicator::freeBuffer(Communicator::buffer_type buffer) { auto& buffer_handler = buffer_handlers_m.get(); - return buffer_handler.freeBuffer(buffer); + buffer_handler.freeBuffer(buffer); } } // namespace mpi diff --git a/src/Communicate/CommunicatorLogging.cpp b/src/Communicate/CommunicatorLogging.cpp index 62237459a..ae2a2376c 100644 --- a/src/Communicate/CommunicatorLogging.cpp +++ b/src/Communicate/CommunicatorLogging.cpp @@ -94,9 +94,7 @@ namespace ippl { Inform logFile(0, filename.c_str(), Inform::OVERWRITE, 0); logFile.setOutputLevel(1); - logFile << "Timestamp,Method,Rank,MemorySpace,AllocatedSize,FreeSize,Parameters" << endl; - std::cout << "hello world" << std::endl; - std::cout << allLogs.size() << std::endl; + logFile << "Timestamp,Method,Rank,MemorySpace,usedSize,FreeSize,Parameters" << endl; for (const auto& log : allLogs) { auto timestamp = std::chrono::duration_cast( @@ -104,7 +102,7 @@ namespace ippl { .count(); logFile << timestamp << "," << log.methodName << "," << log.rank << "," << log.memorySpace << "," - << log.allocatedSize << "," << log.freeSize; + << log.usedSize << "," << log.freeSize; logFile << ",\""; bool first = true; diff --git a/src/Communicate/LogEntry.cpp b/src/Communicate/LogEntry.cpp index a33d38aaa..444221d6d 100644 --- a/src/Communicate/LogEntry.cpp +++ b/src/Communicate/LogEntry.cpp @@ -1,62 +1,67 @@ #include "Communicate/LogEntry.h" -void serializeString(std::vector& buffer, const std::string& str) { - size_t length = str.size(); - serializeBasicType(buffer, length); // First, serialize the length of the string - buffer.insert(buffer.end(), str.begin(), str.end()); // Then, serialize the string itself -} - -std::string deserializeString(const std::vector& buffer, size_t& offset) { - size_t length = deserializeBasicType(buffer, offset); // Get the length of the string - std::string str(buffer.begin() + offset, buffer.begin() + offset + length); // Extract the string - offset += length; - return str; -} - -std::vector LogEntry::serialize() const { - std::vector buffer; - - serializeString(buffer, methodName); - serializeBasicType(buffer, allocatedSize); - serializeBasicType(buffer, freeSize); - serializeString(buffer, memorySpace); - serializeBasicType(buffer, rank); - - // Serialize the timestamp (as duration since epoch) - auto duration = timestamp.time_since_epoch().count(); - serializeBasicType(buffer, duration); - - size_t mapSize = parameters.size(); - serializeBasicType(buffer, mapSize); - for (const auto& pair : parameters) { - serializeString(buffer, pair.first); - serializeString(buffer, pair.second); +namespace ippl { + + void serializeString(std::vector& buffer, const std::string& str) { + size_t length = str.size(); + serializeBasicType(buffer, length); // First, serialize the length of the string + buffer.insert(buffer.end(), str.begin(), str.end()); // Then, serialize the string itself + } + + std::string deserializeString(const std::vector& buffer, size_t& offset) { + size_t length = + deserializeBasicType(buffer, offset); // Get the length of the string + std::string str(buffer.begin() + offset, + buffer.begin() + offset + length); // Extract the string + offset += length; + return str; } - return buffer; -} - -LogEntry LogEntry::deserialize(const std::vector& buffer, size_t offset) { - LogEntry entry; - size_t current_pos = offset; - - entry.methodName = deserializeString(buffer, current_pos); - entry.allocatedSize = deserializeBasicType(buffer, current_pos); - entry.freeSize = deserializeBasicType(buffer, current_pos); - entry.memorySpace = deserializeString(buffer, current_pos); - entry.rank = deserializeBasicType(buffer, current_pos); - - auto duration = deserializeBasicType(buffer, current_pos); - entry.timestamp = std::chrono::time_point( - std::chrono::high_resolution_clock::duration(duration) - ); - - size_t mapSize = deserializeBasicType(buffer, current_pos); - for (size_t i = 0; i < mapSize; ++i) { - std::string key = deserializeString(buffer, current_pos); - std::string value = deserializeString(buffer, current_pos); - entry.parameters[key] = value; + std::vector LogEntry::serialize() const { + std::vector buffer; + + serializeString(buffer, methodName); + serializeBasicType(buffer, usedSize); + serializeBasicType(buffer, freeSize); + serializeString(buffer, memorySpace); + serializeBasicType(buffer, rank); + + // Serialize the timestamp (as duration since epoch) + auto duration = timestamp.time_since_epoch().count(); + serializeBasicType(buffer, duration); + + size_t mapSize = parameters.size(); + serializeBasicType(buffer, mapSize); + for (const auto& pair : parameters) { + serializeString(buffer, pair.first); + serializeString(buffer, pair.second); + } + + return buffer; + } + + LogEntry LogEntry::deserialize(const std::vector& buffer, size_t offset) { + LogEntry entry; + size_t current_pos = offset; + + entry.methodName = deserializeString(buffer, current_pos); + entry.usedSize = deserializeBasicType(buffer, current_pos); + entry.freeSize = deserializeBasicType(buffer, current_pos); + entry.memorySpace = deserializeString(buffer, current_pos); + entry.rank = deserializeBasicType(buffer, current_pos); + + auto duration = deserializeBasicType(buffer, current_pos); + entry.timestamp = std::chrono::time_point( + std::chrono::high_resolution_clock::duration(duration)); + + size_t mapSize = deserializeBasicType(buffer, current_pos); + for (size_t i = 0; i < mapSize; ++i) { + std::string key = deserializeString(buffer, current_pos); + std::string value = deserializeString(buffer, current_pos); + entry.parameters[key] = value; + } + + return entry; } - return entry; -} +} // namespace ippl diff --git a/src/Communicate/LogEntry.h b/src/Communicate/LogEntry.h index 14a79c361..a853c3431 100644 --- a/src/Communicate/LogEntry.h +++ b/src/Communicate/LogEntry.h @@ -1,38 +1,42 @@ #ifndef IPPL_LOGENTRY_H #define IPPL_LOGENTRY_H -#include -#include -#include #include #include +#include +#include +#include + +namespace ippl { + + struct LogEntry { + std::string methodName; + std::map parameters; + size_t usedSize; + size_t freeSize; + std::string memorySpace; + int rank; + std::chrono::time_point timestamp; + + std::vector serialize() const; + static LogEntry deserialize(const std::vector& buffer, size_t offset = 0); + }; + + template + void serializeBasicType(std::vector& buffer, const T& value) { + size_t size = sizeof(T); + buffer.resize(buffer.size() + size); + std::memcpy(buffer.data() + buffer.size() - size, &value, size); + } + + template + T deserializeBasicType(const std::vector& buffer, size_t& offset) { + T value; + std::memcpy(&value, buffer.data() + offset, sizeof(T)); + offset += sizeof(T); + return value; + } -struct LogEntry { - std::string methodName; - std::map parameters; - size_t allocatedSize; - size_t freeSize; - std::string memorySpace; - int rank; - std::chrono::time_point timestamp; - - std::vector serialize() const; - static LogEntry deserialize(const std::vector& buffer, size_t offset = 0); -}; - -template -void serializeBasicType(std::vector& buffer, const T& value) { - size_t size = sizeof(T); - buffer.resize(buffer.size() + size); - std::memcpy(buffer.data() + buffer.size() - size, &value, size); -} - -template -T deserializeBasicType(const std::vector& buffer, size_t& offset) { - T value; - std::memcpy(&value, buffer.data() + offset, sizeof(T)); - offset += sizeof(T); - return value; -} +} // namespace ippl #endif diff --git a/src/Communicate/LoggingBufferHandler.h b/src/Communicate/LoggingBufferHandler.h index 96abde583..1b56e68aa 100644 --- a/src/Communicate/LoggingBufferHandler.h +++ b/src/Communicate/LoggingBufferHandler.h @@ -1,119 +1,128 @@ #ifndef IPPL_LOGGING_BUFFER_HANDLER_H #define IPPL_LOGGING_BUFFER_HANDLER_H -#include -#include #include -#include #include +#include +#include +#include + #include "Communicate/BufferHandler.h" #include "Communicate/LogEntry.h" -/** - * @class LoggingBufferHandler - * @brief Decorator class for buffer management that adds logging capabilities to buffer operations. - * - * `LoggingBufferHandler` extends the basic functionality of `IBufferHandler` by recording - * detailed logs of buffer operations, such as allocation, deallocation, and resizing. - * This allows tracking of memory usage patterns and provides a record of buffer activity. - * - * @tparam MemorySpace The memory space in which buffers are managed (e.g., HostSpace, CudaSpace). - * - * This class is a decorator for `BufferHandler` and does not modify buffer management logic. - * Instead, it adds logging for monitoring purposes. - */ -template -class LoggingBufferHandler : public IBufferHandler { -public: - using buffer_type = typename IBufferHandler::buffer_type; - using size_type = typename IBufferHandler::size_type; - - /** - * @brief Constructs a LoggingBufferHandler with an existing buffer handler. - * @param handler A shared pointer to an `IBufferHandler` instance used for buffer operations. - * @param rank The MPI rank for logging purposes, used to identify the source of logs. - */ - LoggingBufferHandler(std::shared_ptr> handler, int rank); +namespace ippl { /** - * @brief Default constructor, creates an internal `BufferHandler` for managing buffers. - * This constructor also initializes the rank by calling `MPI_Comm_rank`. - */ - LoggingBufferHandler(); - - /** - * @brief Allocates or retrieves a buffer and logs the action. + * @class LoggingBufferHandler + * @brief Decorator class for buffer management that adds logging capabilities to buffer + * operations. * - * Overrides `IBufferHandler::getBuffer`, providing the same buffer allocation behavior - * while recording an entry in the log with the operation details. + * `LoggingBufferHandler` extends the basic functionality of `IBufferHandler` by recording + * detailed logs of buffer operations, such as allocation, deallocation, and resizing. + * This allows tracking of memory usage patterns and provides a record of buffer activity. * - * @param size Requested size of the buffer. - * @param overallocation Optional multiplier to allocate extra buffer space. - * @return A buffer object. - */ - buffer_type getBuffer(size_type size, double overallocation) override; - - /** - * @brief Frees a buffer and logs the action. - * - * Overrides `IBufferHandler::freeBuffer`. Calls `BufferHandler::freeBuffer` and records the - * operation in the log. + * @tparam MemorySpace The memory space in which buffers are managed (e.g., HostSpace, + * CudaSpace). * - * @param buffer The buffer to be freed. + * This class is a decorator for `BufferHandler` and does not modify buffer management logic. + * Instead, it adds logging for monitoring purposes. */ - void freeBuffer(buffer_type buffer) override; + template + class LoggingBufferHandler : public IBufferHandler { + public: + using buffer_type = typename IBufferHandler::buffer_type; + using size_type = typename IBufferHandler::size_type; - /** - * @brief Frees all buffers and logs the action. - * - * Overrides `IBufferHandler::freeAllBuffers`. Calls `BufferHandler::freeAllBuffers` and logs - * the operation. - */ - void freeAllBuffers() override; + /** + * @brief Constructs a LoggingBufferHandler with an existing buffer handler. + * @param handler A shared pointer to an `IBufferHandler` instance used for buffer + * operations. + * @param rank The MPI rank for logging purposes, used to identify the source of logs. + */ + LoggingBufferHandler(std::shared_ptr> handler, int rank); - /** - * @brief Deletes all buffers and logs the action. - * - * Overrides `IBufferHandler::deleteAllBuffers`. Calls `BufferHandler::deleteAllBuffers` and - * logs the operation. - */ - void deleteAllBuffers() override; + /** + * @brief Default constructor, creates an internal `BufferHandler` for managing buffers. + * This constructor also initializes the rank by calling `MPI_Comm_rank`. + */ + LoggingBufferHandler(); - /** - * @brief Retrieves the total size of allocated buffers. - * @return The size of allocated buffers. - */ - size_type getAllocatedSize() const override; + /** + * @brief Allocates or retrieves a buffer and logs the action. + * + * Overrides `IBufferHandler::getBuffer`, providing the same buffer allocation behavior + * while recording an entry in the log with the operation details. + * + * @param size Requested size of the buffer. + * @param overallocation Optional multiplier to allocate extra buffer space. + * @return A buffer object. + */ + buffer_type getBuffer(size_type size, double overallocation) override; - /** - * @brief Retrieves the total size of free buffers. - * @return The size of free buffers. - */ - size_type getFreeSize() const override; + /** + * @brief Frees a buffer and logs the action. + * + * Overrides `IBufferHandler::freeBuffer`. Calls `BufferHandler::freeBuffer` and records the + * operation in the log. + * + * @param buffer The buffer to be freed. + */ + void freeBuffer(buffer_type buffer) override; - /** - * @brief Retrieves the list of log entries. - * @return A constant reference to a vector containing log entries. - */ - const std::vector& getLogs() const; + /** + * @brief Frees all buffers and logs the action. + * + * Overrides `IBufferHandler::freeAllBuffers`. Calls `BufferHandler::freeAllBuffers` and + * logs the operation. + */ + void freeAllBuffers() override; -private: - std::shared_ptr> handler_; ///< Internal handler for buffer management. - std::vector logEntries_; ///< Log entries for buffer operations. - int rank_; ///< MPI rank for identifying log sources. + /** + * @brief Deletes all buffers and logs the action. + * + * Overrides `IBufferHandler::deleteAllBuffers`. Calls `BufferHandler::deleteAllBuffers` and + * logs the operation. + */ + void deleteAllBuffers() override; - /** - * @brief Records a method call in the log with its parameters. - * - * This method creates a log entry with details of the buffer operation, - * including the method name, parameters, allocated size, free size, - * memory space, rank, and timestamp. - * - * @param methodName Name of the method being logged (e.g., "getBuffer"). - * @param parameters A map of parameter names and values for the operation. - */ - void logMethod(const std::string& methodName, const std::map& parameters); -}; + /** + * @brief Retrieves the total size of allocated buffers. + * @return The size of allocated buffers. + */ + size_type getUsedSize() const override; + + /** + * @brief Retrieves the total size of free buffers. + * @return The size of free buffers. + */ + size_type getFreeSize() const override; + + /** + * @brief Retrieves the list of log entries. + * @return A constant reference to a vector containing log entries. + */ + const std::vector& getLogs() const; + + private: + std::shared_ptr> + handler_m; ///< Internal handler for buffer management. + std::vector logEntries_m; ///< Log entries for buffer operations. + int rank_m; ///< MPI rank for identifying log sources. + + /** + * @brief Records a method call in the log with its parameters. + * + * This method creates a log entry with details of the buffer operation, + * including the method name, parameters, allocated size, free size, + * memory space, rank, and timestamp. + * + * @param methodName Name of the method being logged (e.g., "getBuffer"). + * @param parameters A map of parameter names and values for the operation. + */ + void logMethod(const std::string& methodName, + const std::map& parameters); + }; +} // namespace ippl #include "Communicate/LoggingBufferHandler.hpp" diff --git a/src/Communicate/LoggingBufferHandler.hpp b/src/Communicate/LoggingBufferHandler.hpp index c66fde348..689175905 100644 --- a/src/Communicate/LoggingBufferHandler.hpp +++ b/src/Communicate/LoggingBufferHandler.hpp @@ -1,73 +1,75 @@ #ifndef IPPL_LOGGING_BUFFER_HANDLER_HPP #define IPPL_LOGGING_BUFFER_HANDLER_HPP -#include #include +#include -template -LoggingBufferHandler::LoggingBufferHandler(std::shared_ptr> handler, int rank) - : handler_(std::move(handler)), rank_(rank) {} +namespace ippl { -template -LoggingBufferHandler::LoggingBufferHandler() { - handler_ = std::make_shared>(); - MPI_Comm_rank(MPI_COMM_WORLD, &rank_); -} + template + LoggingBufferHandler::LoggingBufferHandler( + std::shared_ptr> handler, int rank) + : handler_m(std::move(handler)) + , rank_m(rank) {} -template -typename LoggingBufferHandler::buffer_type LoggingBufferHandler::getBuffer( - size_type size, double overallocation) { - auto buffer = handler_->getBuffer(size, overallocation); - logMethod("getBuffer", {{"size", std::to_string(size)}, {"overallocation", std::to_string(overallocation)}}); - return buffer; -} + template + LoggingBufferHandler::LoggingBufferHandler() { + handler_m = std::make_shared>(); + MPI_Comm_rank(MPI_COMM_WORLD, &rank_m); + } -template -void LoggingBufferHandler::freeBuffer(buffer_type buffer) { - handler_->freeBuffer(buffer); - logMethod("freeBuffer", {}); -} + template + typename LoggingBufferHandler::buffer_type + LoggingBufferHandler::getBuffer(size_type size, double overallocation) { + auto buffer = handler_m->getBuffer(size, overallocation); + logMethod("getBuffer", {{"size", std::to_string(size)}, + {"overallocation", std::to_string(overallocation)}}); + return buffer; + } -template -void LoggingBufferHandler::freeAllBuffers() { - handler_->freeAllBuffers(); - logMethod("freeAllBuffers", {}); -} + template + void LoggingBufferHandler::freeBuffer(buffer_type buffer) { + handler_m->freeBuffer(buffer); + logMethod("freeBuffer", {}); + } -template -void LoggingBufferHandler::deleteAllBuffers() { - handler_->deleteAllBuffers(); - logMethod("deleteAllBuffers", {}); -} + template + void LoggingBufferHandler::freeAllBuffers() { + handler_m->freeAllBuffers(); + logMethod("freeAllBuffers", {}); + } -template -typename LoggingBufferHandler::size_type LoggingBufferHandler::getAllocatedSize() const { - return handler_->getAllocatedSize(); -} + template + void LoggingBufferHandler::deleteAllBuffers() { + handler_m->deleteAllBuffers(); + logMethod("deleteAllBuffers", {}); + } -template -typename LoggingBufferHandler::size_type LoggingBufferHandler::getFreeSize() const { - return handler_->getFreeSize(); -} + template + typename LoggingBufferHandler::size_type + LoggingBufferHandler::getUsedSize() const { + return handler_m->getUsedSize(); + } -template -const std::vector& LoggingBufferHandler::getLogs() const { - std::cout << logEntries_.size() << std::endl; - return logEntries_; -} + template + typename LoggingBufferHandler::size_type + LoggingBufferHandler::getFreeSize() const { + return handler_m->getFreeSize(); + } -template -void LoggingBufferHandler::logMethod(const std::string& methodName, const std::map& parameters) { - logEntries_.push_back({ - methodName, - parameters, - handler_->getAllocatedSize(), - handler_->getFreeSize(), - MemorySpace::name(), - rank_, - std::chrono::high_resolution_clock::now() - }); -} + template + const std::vector& LoggingBufferHandler::getLogs() const { + return logEntries_m; + } -#endif + template + void LoggingBufferHandler::logMethod( + const std::string& methodName, const std::map& parameters) { + logEntries_m.push_back({methodName, parameters, handler_m->getUsedSize(), + handler_m->getFreeSize(), MemorySpace::name(), rank_m, + std::chrono::high_resolution_clock::now()}); + } +} // namespace ippl + +#endif diff --git a/test/field/TestHalo.cpp b/test/field/TestHalo.cpp index f7f651d3f..71df004b0 100644 --- a/test/field/TestHalo.cpp +++ b/test/field/TestHalo.cpp @@ -145,7 +145,7 @@ int main(int argc, char* argv[]) { IpplTimings::print(); IpplTimings::print(std::string("timing.dat")); - layout.comm.printLogs("testfile.csv"); + layout.comm.printLogs("buffer_memory_logging.csv"); } ippl::finalize(); diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp index 3c9b3e4f0..f340f9925 100644 --- a/unit_tests/Communicate/BufferHandler.cpp +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -13,10 +13,10 @@ class TypedBufferHandlerTest : public ::testing::Test { protected: using memory_space = MemorySpace; - class TestableBufferHandler : public BufferHandler { + class TestableBufferHandler : public ippl::BufferHandler { public: - using BufferHandler::deleteAllBuffers; - using BufferHandler::freeAllBuffers; + using ippl::BufferHandler::deleteAllBuffers; + using ippl::BufferHandler::freeAllBuffers; size_t usedBuffersSize() const { return this->used_buffers.size(); } @@ -125,13 +125,13 @@ TYPED_TEST(TypedBufferHandlerTest, GetBuffer_ZeroSize) { } TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_EmptyHandler) { - EXPECT_EQ(this->handler->getAllocatedSize(), 0); + EXPECT_EQ(this->handler->getUsedSize(), 0); EXPECT_EQ(this->handler->getFreeSize(), 0); } TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterBufferAllocation) { auto buffer = this->handler->getBuffer(100, 1.0); - EXPECT_EQ(this->handler->getAllocatedSize(), 100); + EXPECT_EQ(this->handler->getUsedSize(), 100); EXPECT_EQ(this->handler->getFreeSize(), 0); } @@ -139,7 +139,7 @@ TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeBuffer) { auto buffer = this->handler->getBuffer(100, 1.0); this->handler->freeBuffer(buffer); - EXPECT_EQ(this->handler->getAllocatedSize(), 0); + EXPECT_EQ(this->handler->getUsedSize(), 0); EXPECT_EQ(this->handler->getFreeSize(), 100); } @@ -149,7 +149,7 @@ TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeAllBuffers) this->handler->freeAllBuffers(); - EXPECT_EQ(this->handler->getAllocatedSize(), 0); + EXPECT_EQ(this->handler->getUsedSize(), 0); EXPECT_EQ(this->handler->getFreeSize(), 150); } @@ -159,7 +159,7 @@ TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterDeleteAllBuffers this->handler->deleteAllBuffers(); - EXPECT_EQ(this->handler->getAllocatedSize(), 0); + EXPECT_EQ(this->handler->getUsedSize(), 0); EXPECT_EQ(this->handler->getFreeSize(), 0); } @@ -169,7 +169,7 @@ TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_ResizeBufferLargerTha auto largeBuffer = this->handler->getBuffer(200, 1.0); - EXPECT_EQ(this->handler->getAllocatedSize(), 200); + EXPECT_EQ(this->handler->getUsedSize(), 200); EXPECT_EQ(this->handler->getFreeSize(), 0); } diff --git a/unit_tests/Communicate/LogEntry.cpp b/unit_tests/Communicate/LogEntry.cpp index 4ad27eac0..0d14012e5 100644 --- a/unit_tests/Communicate/LogEntry.cpp +++ b/unit_tests/Communicate/LogEntry.cpp @@ -5,24 +5,24 @@ #include "TestUtils.h" #include "gtest/gtest.h" -LogEntry createSampleLogEntry() { - LogEntry logEntry; - logEntry.methodName = "TestMethod"; - logEntry.allocatedSize = 1024; - logEntry.freeSize = 512; +ippl::LogEntry createSampleLogEntry() { + ippl::LogEntry logEntry; + logEntry.methodName = "TestMethod"; + logEntry.usedSize = 1024; + logEntry.freeSize = 512; logEntry.memorySpace = "HostSpace"; - logEntry.rank = 1; - logEntry.timestamp = std::chrono::high_resolution_clock::now(); + logEntry.rank = 1; + logEntry.timestamp = std::chrono::high_resolution_clock::now(); logEntry.parameters["overallocation"] = "1.5"; - logEntry.parameters["buffer_size"] = "2048"; + logEntry.parameters["buffer_size"] = "2048"; return logEntry; } TEST(LogEntryTest, Serialize) { - LogEntry logEntry = createSampleLogEntry(); - + ippl::LogEntry logEntry = createSampleLogEntry(); + std::vector buffer; buffer = logEntry.serialize(); @@ -30,47 +30,51 @@ TEST(LogEntryTest, Serialize) { } TEST(LogEntryTest, Deserialize) { - LogEntry logEntry = createSampleLogEntry(); + ippl::LogEntry logEntry = createSampleLogEntry(); std::vector buffer; buffer = logEntry.serialize(); - LogEntry deserializedLogEntry = LogEntry::deserialize(buffer); + ippl::LogEntry deserializedLogEntry = ippl::LogEntry::deserialize(buffer); EXPECT_EQ(deserializedLogEntry.methodName, logEntry.methodName); - EXPECT_EQ(deserializedLogEntry.allocatedSize, logEntry.allocatedSize); + EXPECT_EQ(deserializedLogEntry.usedSize, logEntry.usedSize); EXPECT_EQ(deserializedLogEntry.freeSize, logEntry.freeSize); EXPECT_EQ(deserializedLogEntry.memorySpace, logEntry.memorySpace); EXPECT_EQ(deserializedLogEntry.rank, logEntry.rank); EXPECT_EQ(deserializedLogEntry.parameters.size(), logEntry.parameters.size()); - EXPECT_EQ(deserializedLogEntry.parameters.at("overallocation"), logEntry.parameters.at("overallocation")); - EXPECT_EQ(deserializedLogEntry.parameters.at("buffer_size"), logEntry.parameters.at("buffer_size")); + EXPECT_EQ(deserializedLogEntry.parameters.at("overallocation"), + logEntry.parameters.at("overallocation")); + EXPECT_EQ(deserializedLogEntry.parameters.at("buffer_size"), + logEntry.parameters.at("buffer_size")); - auto originalTime = logEntry.timestamp.time_since_epoch().count(); + auto originalTime = logEntry.timestamp.time_since_epoch().count(); auto deserializedTime = deserializedLogEntry.timestamp.time_since_epoch().count(); EXPECT_EQ(originalTime, deserializedTime); } TEST(LogEntryTest, RoundTripSerialization) { - LogEntry logEntry = createSampleLogEntry(); + ippl::LogEntry logEntry = createSampleLogEntry(); std::vector buffer; buffer = logEntry.serialize(); - LogEntry deserializedLogEntry = LogEntry::deserialize(buffer); + ippl::LogEntry deserializedLogEntry = ippl::LogEntry::deserialize(buffer); EXPECT_EQ(deserializedLogEntry.methodName, logEntry.methodName); - EXPECT_EQ(deserializedLogEntry.allocatedSize, logEntry.allocatedSize); + EXPECT_EQ(deserializedLogEntry.usedSize, logEntry.usedSize); EXPECT_EQ(deserializedLogEntry.freeSize, logEntry.freeSize); EXPECT_EQ(deserializedLogEntry.memorySpace, logEntry.memorySpace); EXPECT_EQ(deserializedLogEntry.rank, logEntry.rank); EXPECT_EQ(deserializedLogEntry.parameters.size(), logEntry.parameters.size()); - EXPECT_EQ(deserializedLogEntry.parameters.at("overallocation"), logEntry.parameters.at("overallocation")); - EXPECT_EQ(deserializedLogEntry.parameters.at("buffer_size"), logEntry.parameters.at("buffer_size")); + EXPECT_EQ(deserializedLogEntry.parameters.at("overallocation"), + logEntry.parameters.at("overallocation")); + EXPECT_EQ(deserializedLogEntry.parameters.at("buffer_size"), + logEntry.parameters.at("buffer_size")); - auto originalTime = logEntry.timestamp.time_since_epoch().count(); + auto originalTime = logEntry.timestamp.time_since_epoch().count(); auto deserializedTime = deserializedLogEntry.timestamp.time_since_epoch().count(); EXPECT_EQ(originalTime, deserializedTime); } diff --git a/unit_tests/Communicate/LoggingBufferHandler.cpp b/unit_tests/Communicate/LoggingBufferHandler.cpp index e72ee5eb0..57bd36d6c 100644 --- a/unit_tests/Communicate/LoggingBufferHandler.cpp +++ b/unit_tests/Communicate/LoggingBufferHandler.cpp @@ -12,14 +12,14 @@ class TypedLoggingBufferHandlerTest : public ::testing::Test { protected: void SetUp() override { rank = 0; - this->bufferHandler = std::make_shared>(); + this->bufferHandler = std::make_shared>(); this->loggingHandler = - std::make_shared>(bufferHandler, rank); + std::make_shared>(bufferHandler, rank); } int rank; - std::shared_ptr> bufferHandler; - std::shared_ptr> loggingHandler; + std::shared_ptr> bufferHandler; + std::shared_ptr> loggingHandler; }; TYPED_TEST_SUITE(TypedLoggingBufferHandlerTest, MemorySpaces); @@ -52,7 +52,7 @@ TYPED_TEST(TypedLoggingBufferHandlerTest, GetBufferLogsCorrectly) { std::string overallocationStr = entry.parameters.at("overallocation"); compareNumericParameter(overallocationStr, 1.5); - EXPECT_EQ(entry.allocatedSize, this->bufferHandler->getAllocatedSize()); + EXPECT_EQ(entry.usedSize, this->bufferHandler->getUsedSize()); EXPECT_EQ(entry.freeSize, this->bufferHandler->getFreeSize()); EXPECT_EQ(entry.memorySpace, TypeParam::name()); EXPECT_EQ(entry.rank, this->rank); @@ -67,7 +67,7 @@ TYPED_TEST(TypedLoggingBufferHandlerTest, FreeBufferLogsCorrectly) { const auto& entry = logs[1]; EXPECT_EQ(entry.methodName, "freeBuffer"); - EXPECT_EQ(entry.allocatedSize, this->bufferHandler->getAllocatedSize()); + EXPECT_EQ(entry.usedSize, this->bufferHandler->getUsedSize()); EXPECT_EQ(entry.freeSize, this->bufferHandler->getFreeSize()); EXPECT_EQ(entry.memorySpace, TypeParam::name()); EXPECT_EQ(entry.rank, this->rank); @@ -83,7 +83,7 @@ TYPED_TEST(TypedLoggingBufferHandlerTest, FreeAllBuffersLogsCorrectly) { const auto& entry = logs[2]; EXPECT_EQ(entry.methodName, "freeAllBuffers"); - EXPECT_EQ(entry.allocatedSize, this->bufferHandler->getAllocatedSize()); + EXPECT_EQ(entry.usedSize, this->bufferHandler->getUsedSize()); EXPECT_EQ(entry.freeSize, this->bufferHandler->getFreeSize()); EXPECT_EQ(entry.memorySpace, TypeParam::name()); EXPECT_EQ(entry.rank, this->rank); @@ -99,7 +99,7 @@ TYPED_TEST(TypedLoggingBufferHandlerTest, DeleteAllBuffersLogsCorrectly) { const auto& entry = logs[2]; EXPECT_EQ(entry.methodName, "deleteAllBuffers"); - EXPECT_EQ(entry.allocatedSize, this->bufferHandler->getAllocatedSize()); + EXPECT_EQ(entry.usedSize, this->bufferHandler->getUsedSize()); EXPECT_EQ(entry.freeSize, this->bufferHandler->getFreeSize()); EXPECT_EQ(entry.memorySpace, TypeParam::name()); EXPECT_EQ(entry.rank, this->rank); From 50ab147f27bef08fe16be29345a0636ff1f2d2b4 Mon Sep 17 00:00:00 2001 From: Jonas Meier Date: Thu, 7 Nov 2024 11:08:16 +0100 Subject: [PATCH 21/21] Renamed BufferHandler and added comments for tests --- src/Communicate/BufferHandler.h | 59 ++++++++----------- src/Communicate/BufferHandler.hpp | 36 +++++------ src/Communicate/LoggingBufferHandler.h | 26 ++++---- src/Communicate/LoggingBufferHandler.hpp | 8 +-- unit_tests/Communicate/BufferHandler.cpp | 35 ++++++++--- unit_tests/Communicate/LogEntry.cpp | 29 ++------- .../Communicate/LoggingBufferHandler.cpp | 8 ++- 7 files changed, 96 insertions(+), 105 deletions(-) diff --git a/src/Communicate/BufferHandler.h b/src/Communicate/BufferHandler.h index 6c06cd29c..c0400d1ac 100644 --- a/src/Communicate/BufferHandler.h +++ b/src/Communicate/BufferHandler.h @@ -18,13 +18,13 @@ namespace ippl { * @tparam MemorySpace The memory space type used for buffer allocation. */ template - class IBufferHandler { + class BufferHandler { public: using archive_type = ippl::detail::Archive; using buffer_type = std::shared_ptr; using size_type = ippl::detail::size_type; - virtual ~IBufferHandler() {} + virtual ~BufferHandler() {} /** * @brief Requests a memory buffer of a specified size. @@ -67,9 +67,9 @@ namespace ippl { virtual void deleteAllBuffers() = 0; /** - * @brief Gets the size of all allocated buffers. + * @brief Gets the size of all buffers in use. * - * @return Total size of allocated buffers in bytes. + * @return Total size of buffers that are in use in bytes. */ virtual size_type getUsedSize() const = 0; @@ -82,31 +82,32 @@ namespace ippl { }; /** - * @class BufferHandler - * @brief Concrete implementation of IBufferHandler for managing memory buffers. + * @class DefaultBufferHandler + * @brief Concrete implementation of BufferHandler for managing memory buffers. * - * This class implements the IBufferHandler interface, providing concrete behavior for - * buffer allocation, freeing, and memory management. It maintains a pool of allocated - * and free buffers to optimize memory usage. + * This class implements the BufferHandler interface, providing concrete behavior for + * buffer allocation, freeing, and memory management. It maintains two sorted sets of free and + * in-use buffers to allow for efficient queries. * * @tparam MemorySpace The memory space type for the buffer (e.g., `Kokkos::HostSpace`). */ template - class BufferHandler : public IBufferHandler { + class DefaultBufferHandler : public BufferHandler { public: - using typename IBufferHandler::archive_type; - using typename IBufferHandler::buffer_type; - using typename IBufferHandler::size_type; + using typename BufferHandler::archive_type; + using typename BufferHandler::buffer_type; + using typename BufferHandler::size_type; - ~BufferHandler() override; + ~DefaultBufferHandler() override; /** * @brief Acquires a buffer of at least the specified size. * * Requests a memory buffer of the specified size, with the option * to request a buffer larger than the base size by an overallocation - * multiplier. Implementations should attempt to reuse existing - * buffers if possible. + * multiplier. If a sufficiently large buffer is available, it is returned. If not, the + * largest free buffer is reallocated. If there are no free buffers available, only then a + * new buffer is allocated. * * @param size The required buffer size. * @param overallocation A multiplier to allocate additional buffer space. @@ -115,39 +116,27 @@ namespace ippl { buffer_type getBuffer(size_type size, double overallocation) override; /** - * @brief Frees a specified buffer. - * - * Moves the specified buffer to a free state, making it available - * for reuse in future buffer requests. - * - * @param buffer The buffer to be freed. + * @copydoc BufferHandler::freeBuffer */ void freeBuffer(buffer_type buffer) override; /** - * @brief Frees all allocated buffers and adds them to the free pool without deallocating - * the actual memory region. + * @copydoc BufferHandler::freeBuffer */ void freeAllBuffers() override; /** - * @brief Deletes all buffers, thereby freeing the memory. + * @copydoc BufferHandler::freeBuffer */ void deleteAllBuffers() override; /** - * @brief Retrieves the total allocated size, i.e. the size of allocated memory currently in - * use. - * - * @return The total size of allocated buffers in use. + * @copydoc BufferHandler::freeBuffer */ size_type getUsedSize() const override; /** - * @brief Retrieves the total size of free buffers, i.e. size of allocated memory currently - * not in use. - * - * @return The total size of free buffers. + * @copydoc BufferHandler::freeBuffer */ size_type getFreeSize() const override; @@ -170,9 +159,9 @@ namespace ippl { protected: buffer_set_type used_buffers{ - &BufferHandler::bufferSizeComparator}; ///< Set of used buffers + &DefaultBufferHandler::bufferSizeComparator}; ///< Set of used buffers buffer_set_type free_buffers{ - &BufferHandler::bufferSizeComparator}; ///< Set of free buffers + &DefaultBufferHandler::bufferSizeComparator}; ///< Set of free buffers }; } // namespace ippl diff --git a/src/Communicate/BufferHandler.hpp b/src/Communicate/BufferHandler.hpp index 552c8d7e3..012f7521b 100644 --- a/src/Communicate/BufferHandler.hpp +++ b/src/Communicate/BufferHandler.hpp @@ -4,10 +4,10 @@ namespace ippl { template - BufferHandler::~BufferHandler() {} + DefaultBufferHandler::~DefaultBufferHandler() {} template - typename BufferHandler::buffer_type BufferHandler::getBuffer( + typename DefaultBufferHandler::buffer_type DefaultBufferHandler::getBuffer( size_type size, double overallocation) { size_type requiredSize = static_cast(size * overallocation); @@ -24,14 +24,14 @@ namespace ippl { } template - void BufferHandler::freeBuffer(buffer_type buffer) { + void DefaultBufferHandler::freeBuffer(buffer_type buffer) { if (isBufferUsed(buffer)) { releaseUsedBuffer(buffer); } } template - void BufferHandler::freeAllBuffers() { + void DefaultBufferHandler::freeAllBuffers() { free_buffers.insert(used_buffers.begin(), used_buffers.end()); used_buffers.clear(); @@ -40,7 +40,7 @@ namespace ippl { } template - void BufferHandler::deleteAllBuffers() { + void DefaultBufferHandler::deleteAllBuffers() { freeSize_m = 0; usedSize_m = 0; @@ -49,18 +49,18 @@ namespace ippl { } template - typename BufferHandler::size_type BufferHandler::getUsedSize() + typename DefaultBufferHandler::size_type DefaultBufferHandler::getUsedSize() const { return usedSize_m; } template - typename BufferHandler::size_type BufferHandler::getFreeSize() const { + typename DefaultBufferHandler::size_type DefaultBufferHandler::getFreeSize() const { return freeSize_m; } template - bool BufferHandler::bufferSizeComparator(const buffer_type& lhs, + bool DefaultBufferHandler::bufferSizeComparator(const buffer_type& lhs, const buffer_type& rhs) { if (lhs->getBufferSize() != rhs->getBufferSize()) { return lhs->getBufferSize() < rhs->getBufferSize(); @@ -71,12 +71,12 @@ namespace ippl { } template - bool BufferHandler::isBufferUsed(buffer_type buffer) const { + bool DefaultBufferHandler::isBufferUsed(buffer_type buffer) const { return used_buffers.find(buffer) != used_buffers.end(); } template - void BufferHandler::releaseUsedBuffer(buffer_type buffer) { + void DefaultBufferHandler::releaseUsedBuffer(buffer_type buffer) { auto it = used_buffers.find(buffer); usedSize_m -= buffer->getBufferSize(); @@ -87,7 +87,7 @@ namespace ippl { } template - typename BufferHandler::buffer_type BufferHandler::findFreeBuffer( + typename DefaultBufferHandler::buffer_type DefaultBufferHandler::findFreeBuffer( size_type requiredSize) { auto it = findSmallestSufficientBuffer(requiredSize); if (it != free_buffers.end()) { @@ -97,8 +97,8 @@ namespace ippl { } template - typename BufferHandler::buffer_set_type::iterator - BufferHandler::findSmallestSufficientBuffer(size_type requiredSize) { + typename DefaultBufferHandler::buffer_set_type::iterator + DefaultBufferHandler::findSmallestSufficientBuffer(size_type requiredSize) { return std::find_if(free_buffers.begin(), free_buffers.end(), [requiredSize](const buffer_type& buffer) { return buffer->getBufferSize() >= requiredSize; @@ -106,8 +106,8 @@ namespace ippl { } template - typename BufferHandler::buffer_type - BufferHandler::getFreeBuffer(buffer_type buffer) { + typename DefaultBufferHandler::buffer_type + DefaultBufferHandler::getFreeBuffer(buffer_type buffer) { freeSize_m -= buffer->getBufferSize(); usedSize_m += buffer->getBufferSize(); @@ -117,8 +117,8 @@ namespace ippl { } template - typename BufferHandler::buffer_type - BufferHandler::reallocateLargestFreeBuffer(size_type requiredSize) { + typename DefaultBufferHandler::buffer_type + DefaultBufferHandler::reallocateLargestFreeBuffer(size_type requiredSize) { auto largest_it = std::prev(free_buffers.end()); buffer_type buffer = *largest_it; @@ -133,7 +133,7 @@ namespace ippl { } template - typename BufferHandler::buffer_type BufferHandler::allocateNewBuffer( + typename DefaultBufferHandler::buffer_type DefaultBufferHandler::allocateNewBuffer( size_type requiredSize) { buffer_type newBuffer = std::make_shared(requiredSize); diff --git a/src/Communicate/LoggingBufferHandler.h b/src/Communicate/LoggingBufferHandler.h index 1b56e68aa..f6a0bd5d6 100644 --- a/src/Communicate/LoggingBufferHandler.h +++ b/src/Communicate/LoggingBufferHandler.h @@ -17,7 +17,7 @@ namespace ippl { * @brief Decorator class for buffer management that adds logging capabilities to buffer * operations. * - * `LoggingBufferHandler` extends the basic functionality of `IBufferHandler` by recording + * `LoggingBufferHandler` extends the basic functionality of `BufferHandler` by recording * detailed logs of buffer operations, such as allocation, deallocation, and resizing. * This allows tracking of memory usage patterns and provides a record of buffer activity. * @@ -28,18 +28,18 @@ namespace ippl { * Instead, it adds logging for monitoring purposes. */ template - class LoggingBufferHandler : public IBufferHandler { + class LoggingBufferHandler : public BufferHandler { public: - using buffer_type = typename IBufferHandler::buffer_type; - using size_type = typename IBufferHandler::size_type; + using buffer_type = typename BufferHandler::buffer_type; + using size_type = typename BufferHandler::size_type; /** * @brief Constructs a LoggingBufferHandler with an existing buffer handler. - * @param handler A shared pointer to an `IBufferHandler` instance used for buffer + * @param handler A shared pointer to an `BufferHandler` instance used for buffer * operations. * @param rank The MPI rank for logging purposes, used to identify the source of logs. */ - LoggingBufferHandler(std::shared_ptr> handler, int rank); + LoggingBufferHandler(std::shared_ptr> handler, int rank); /** * @brief Default constructor, creates an internal `BufferHandler` for managing buffers. @@ -50,7 +50,7 @@ namespace ippl { /** * @brief Allocates or retrieves a buffer and logs the action. * - * Overrides `IBufferHandler::getBuffer`, providing the same buffer allocation behavior + * Overrides `BufferHandler::getBuffer`, providing the same buffer allocation behavior * while recording an entry in the log with the operation details. * * @param size Requested size of the buffer. @@ -62,7 +62,7 @@ namespace ippl { /** * @brief Frees a buffer and logs the action. * - * Overrides `IBufferHandler::freeBuffer`. Calls `BufferHandler::freeBuffer` and records the + * Overrides `BufferHandler::freeBuffer`. Calls `BufferHandler::freeBuffer` and records the * operation in the log. * * @param buffer The buffer to be freed. @@ -72,7 +72,7 @@ namespace ippl { /** * @brief Frees all buffers and logs the action. * - * Overrides `IBufferHandler::freeAllBuffers`. Calls `BufferHandler::freeAllBuffers` and + * Overrides `BufferHandler::freeAllBuffers`. Calls `BufferHandler::freeAllBuffers` and * logs the operation. */ void freeAllBuffers() override; @@ -80,7 +80,7 @@ namespace ippl { /** * @brief Deletes all buffers and logs the action. * - * Overrides `IBufferHandler::deleteAllBuffers`. Calls `BufferHandler::deleteAllBuffers` and + * Overrides `BufferHandler::deleteAllBuffers`. Calls `BufferHandler::deleteAllBuffers` and * logs the operation. */ void deleteAllBuffers() override; @@ -104,10 +104,10 @@ namespace ippl { const std::vector& getLogs() const; private: - std::shared_ptr> - handler_m; ///< Internal handler for buffer management. + std::shared_ptr> + handler_m; ///< Internal handler for buffer management. std::vector logEntries_m; ///< Log entries for buffer operations. - int rank_m; ///< MPI rank for identifying log sources. + int rank_m; ///< MPI rank for identifying log sources. /** * @brief Records a method call in the log with its parameters. diff --git a/src/Communicate/LoggingBufferHandler.hpp b/src/Communicate/LoggingBufferHandler.hpp index 689175905..33f4269cd 100644 --- a/src/Communicate/LoggingBufferHandler.hpp +++ b/src/Communicate/LoggingBufferHandler.hpp @@ -8,13 +8,13 @@ namespace ippl { template LoggingBufferHandler::LoggingBufferHandler( - std::shared_ptr> handler, int rank) + std::shared_ptr> handler, int rank) : handler_m(std::move(handler)) , rank_m(rank) {} template LoggingBufferHandler::LoggingBufferHandler() { - handler_m = std::make_shared>(); + handler_m = std::make_shared>(); MPI_Comm_rank(MPI_COMM_WORLD, &rank_m); } @@ -66,8 +66,8 @@ namespace ippl { void LoggingBufferHandler::logMethod( const std::string& methodName, const std::map& parameters) { logEntries_m.push_back({methodName, parameters, handler_m->getUsedSize(), - handler_m->getFreeSize(), MemorySpace::name(), rank_m, - std::chrono::high_resolution_clock::now()}); + handler_m->getFreeSize(), MemorySpace::name(), rank_m, + std::chrono::high_resolution_clock::now()}); } } // namespace ippl diff --git a/unit_tests/Communicate/BufferHandler.cpp b/unit_tests/Communicate/BufferHandler.cpp index f340f9925..2c97d9648 100644 --- a/unit_tests/Communicate/BufferHandler.cpp +++ b/unit_tests/Communicate/BufferHandler.cpp @@ -1,8 +1,9 @@ #include "Ippl.h" -#include "Utility/TypeUtils.h" #include "Communicate/BufferHandler.h" +#include "Utility/TypeUtils.h" + #include "TestUtils.h" #include "gtest/gtest.h" @@ -13,10 +14,10 @@ class TypedBufferHandlerTest : public ::testing::Test { protected: using memory_space = MemorySpace; - class TestableBufferHandler : public ippl::BufferHandler { + class TestableBufferHandler : public ippl::DefaultBufferHandler { public: - using ippl::BufferHandler::deleteAllBuffers; - using ippl::BufferHandler::freeAllBuffers; + using ippl::DefaultBufferHandler::deleteAllBuffers; + using ippl::DefaultBufferHandler::freeAllBuffers; size_t usedBuffersSize() const { return this->used_buffers.size(); } @@ -32,6 +33,7 @@ class TypedBufferHandlerTest : public ::testing::Test { TYPED_TEST_CASE(TypedBufferHandlerTest, MemorySpaces); +// Test: Allocating a buffer when no free buffers are available TYPED_TEST(TypedBufferHandlerTest, GetBuffer_EmptyFreeBuffers) { auto buffer = this->handler->getBuffer(100, 1.0); ASSERT_NE(buffer, nullptr); @@ -40,6 +42,7 @@ TYPED_TEST(TypedBufferHandlerTest, GetBuffer_EmptyFreeBuffers) { EXPECT_EQ(this->handler->freeBuffersSize(), 0); } +// Test: Allocating a buffer when a suitable free buffer is available TYPED_TEST(TypedBufferHandlerTest, GetBuffer_SuitableBufferAvailable) { auto buffer1 = this->handler->getBuffer(50, 1.0); this->handler->freeBuffer(buffer1); @@ -49,6 +52,8 @@ TYPED_TEST(TypedBufferHandlerTest, GetBuffer_SuitableBufferAvailable) { EXPECT_EQ(this->handler->usedBuffersSize(), 1); EXPECT_EQ(this->handler->freeBuffersSize(), 0); } + +// Test: Freeing a used buffer moves it to the free buffer pool TYPED_TEST(TypedBufferHandlerTest, FreeBuffer) { auto buffer = this->handler->getBuffer(100, 1.0); this->handler->freeBuffer(buffer); @@ -57,6 +62,7 @@ TYPED_TEST(TypedBufferHandlerTest, FreeBuffer) { EXPECT_EQ(this->handler->freeBuffersSize(), 1); } +// Test: Freeing all used buffers moves them to the free buffer pool TYPED_TEST(TypedBufferHandlerTest, FreeAllBuffers) { auto buffer1 = this->handler->getBuffer(50, 1.0); auto buffer2 = this->handler->getBuffer(100, 1.0); @@ -67,9 +73,11 @@ TYPED_TEST(TypedBufferHandlerTest, FreeAllBuffers) { EXPECT_EQ(this->handler->freeBuffersSize(), 2); } +// Test: Deleting all buffers removes both used and free buffers TYPED_TEST(TypedBufferHandlerTest, DeleteAllBuffers) { this->handler->getBuffer(50, 1.0); - this->handler->getBuffer(100, 1.0); + auto buffer = this->handler->getBuffer(100, 1.0); + this->handler->freeBuffer(buffer); this->handler->deleteAllBuffers(); @@ -77,6 +85,7 @@ TYPED_TEST(TypedBufferHandlerTest, DeleteAllBuffers) { EXPECT_EQ(this->handler->freeBuffersSize(), 0); } +// Test: Allocating a buffer larger than any available free buffer resizes the largest free buffer TYPED_TEST(TypedBufferHandlerTest, GetBuffer_ResizeLargerThanAvailable) { auto smallBuffer = this->handler->getBuffer(50, 1.0); this->handler->freeBuffer(smallBuffer); @@ -87,6 +96,7 @@ TYPED_TEST(TypedBufferHandlerTest, GetBuffer_ResizeLargerThanAvailable) { EXPECT_EQ(this->handler->freeBuffersSize(), 0); } +// Test: Allocating a buffer that matches the size of a free buffer exactly TYPED_TEST(TypedBufferHandlerTest, GetBuffer_ExactSizeMatch) { auto buffer1 = this->handler->getBuffer(100, 1.0); this->handler->freeBuffer(buffer1); @@ -97,15 +107,18 @@ TYPED_TEST(TypedBufferHandlerTest, GetBuffer_ExactSizeMatch) { EXPECT_EQ(this->handler->freeBuffersSize(), 0); } +// Test: Freeing a buffer that does not exist in the used pool has no effect TYPED_TEST(TypedBufferHandlerTest, FreeNonExistentBuffer) { - auto buffer = this->handler->getBuffer(100, 1.0); - auto newBuffer = std::make_shared>(200); + auto buffer = this->handler->getBuffer(100, 1.0); + auto newBuffer = + std::make_shared>(200); this->handler->freeBuffer(newBuffer); EXPECT_EQ(this->handler->usedBuffersSize(), 1); EXPECT_EQ(this->handler->freeBuffersSize(), 0); } +// Test: Repeatedly allocating and freeing buffers should consolidate free buffers TYPED_TEST(TypedBufferHandlerTest, RepeatedAllocateAndFreeCycle) { for (int i = 0; i < 10; ++i) { auto buffer = this->handler->getBuffer(100, 1.0); @@ -116,6 +129,7 @@ TYPED_TEST(TypedBufferHandlerTest, RepeatedAllocateAndFreeCycle) { EXPECT_EQ(this->handler->freeBuffersSize(), 1); } +// Test: Allocating a zero-size buffer should succeed and result in a non-null buffer TYPED_TEST(TypedBufferHandlerTest, GetBuffer_ZeroSize) { auto buffer = this->handler->getBuffer(0, 1.0); ASSERT_NE(buffer, nullptr); @@ -124,17 +138,20 @@ TYPED_TEST(TypedBufferHandlerTest, GetBuffer_ZeroSize) { EXPECT_EQ(this->handler->freeBuffersSize(), 0); } +// Test: The buffer sizes of an empty BufferHandler are zero TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_EmptyHandler) { EXPECT_EQ(this->handler->getUsedSize(), 0); EXPECT_EQ(this->handler->getFreeSize(), 0); } +// Test: Allocating increases used buffer size correctly TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterBufferAllocation) { auto buffer = this->handler->getBuffer(100, 1.0); EXPECT_EQ(this->handler->getUsedSize(), 100); EXPECT_EQ(this->handler->getFreeSize(), 0); } +// Test: Allocating increases used buffer size correctly TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeBuffer) { auto buffer = this->handler->getBuffer(100, 1.0); this->handler->freeBuffer(buffer); @@ -143,6 +160,7 @@ TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeBuffer) { EXPECT_EQ(this->handler->getFreeSize(), 100); } +// Test: Correct size is computed after freeing all buffers TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeAllBuffers) { auto buffer1 = this->handler->getBuffer(50, 1.0); auto buffer2 = this->handler->getBuffer(100, 1.0); @@ -153,6 +171,7 @@ TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterFreeAllBuffers) EXPECT_EQ(this->handler->getFreeSize(), 150); } +// Test: Deleting all buffers results in zero free or used buffer sizes TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterDeleteAllBuffers) { this->handler->getBuffer(50, 1.0); this->handler->getBuffer(100, 1.0); @@ -163,6 +182,7 @@ TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_AfterDeleteAllBuffers EXPECT_EQ(this->handler->getFreeSize(), 0); } +// Test: Buffer size is correctly accounted for if a free buffer is available but we request a larger one, thus reallocating this one TYPED_TEST(TypedBufferHandlerTest, GetAllocatedAndFreeSize_ResizeBufferLargerThanAvailable) { auto smallBuffer = this->handler->getBuffer(50, 1.0); this->handler->freeBuffer(smallBuffer); @@ -179,7 +199,6 @@ int main(int argc, char* argv[]) { { ::testing::InitGoogleTest(&argc, argv); success = RUN_ALL_TESTS(); - } ippl::finalize(); return success; diff --git a/unit_tests/Communicate/LogEntry.cpp b/unit_tests/Communicate/LogEntry.cpp index 0d14012e5..f351cf04d 100644 --- a/unit_tests/Communicate/LogEntry.cpp +++ b/unit_tests/Communicate/LogEntry.cpp @@ -20,6 +20,7 @@ ippl::LogEntry createSampleLogEntry() { return logEntry; } +// Test to ensure LogEntry serialization produces a non-empty buffer TEST(LogEntryTest, Serialize) { ippl::LogEntry logEntry = createSampleLogEntry(); @@ -29,6 +30,7 @@ TEST(LogEntryTest, Serialize) { EXPECT_GT(buffer.size(), 0); } +// Test to ensure LogEntry can be deserialized correctly TEST(LogEntryTest, Deserialize) { ippl::LogEntry logEntry = createSampleLogEntry(); @@ -37,37 +39,14 @@ TEST(LogEntryTest, Deserialize) { ippl::LogEntry deserializedLogEntry = ippl::LogEntry::deserialize(buffer); + // Verify that all fields match the original LogEntry EXPECT_EQ(deserializedLogEntry.methodName, logEntry.methodName); EXPECT_EQ(deserializedLogEntry.usedSize, logEntry.usedSize); EXPECT_EQ(deserializedLogEntry.freeSize, logEntry.freeSize); EXPECT_EQ(deserializedLogEntry.memorySpace, logEntry.memorySpace); EXPECT_EQ(deserializedLogEntry.rank, logEntry.rank); - EXPECT_EQ(deserializedLogEntry.parameters.size(), logEntry.parameters.size()); - EXPECT_EQ(deserializedLogEntry.parameters.at("overallocation"), - logEntry.parameters.at("overallocation")); - EXPECT_EQ(deserializedLogEntry.parameters.at("buffer_size"), - logEntry.parameters.at("buffer_size")); - - auto originalTime = logEntry.timestamp.time_since_epoch().count(); - auto deserializedTime = deserializedLogEntry.timestamp.time_since_epoch().count(); - EXPECT_EQ(originalTime, deserializedTime); -} - -TEST(LogEntryTest, RoundTripSerialization) { - ippl::LogEntry logEntry = createSampleLogEntry(); - - std::vector buffer; - buffer = logEntry.serialize(); - - ippl::LogEntry deserializedLogEntry = ippl::LogEntry::deserialize(buffer); - - EXPECT_EQ(deserializedLogEntry.methodName, logEntry.methodName); - EXPECT_EQ(deserializedLogEntry.usedSize, logEntry.usedSize); - EXPECT_EQ(deserializedLogEntry.freeSize, logEntry.freeSize); - EXPECT_EQ(deserializedLogEntry.memorySpace, logEntry.memorySpace); - EXPECT_EQ(deserializedLogEntry.rank, logEntry.rank); - + // Verify that all parameters are preserved EXPECT_EQ(deserializedLogEntry.parameters.size(), logEntry.parameters.size()); EXPECT_EQ(deserializedLogEntry.parameters.at("overallocation"), logEntry.parameters.at("overallocation")); diff --git a/unit_tests/Communicate/LoggingBufferHandler.cpp b/unit_tests/Communicate/LoggingBufferHandler.cpp index 57bd36d6c..a0db38366 100644 --- a/unit_tests/Communicate/LoggingBufferHandler.cpp +++ b/unit_tests/Communicate/LoggingBufferHandler.cpp @@ -12,13 +12,13 @@ class TypedLoggingBufferHandlerTest : public ::testing::Test { protected: void SetUp() override { rank = 0; - this->bufferHandler = std::make_shared>(); + this->bufferHandler = std::make_shared>(); this->loggingHandler = std::make_shared>(bufferHandler, rank); } int rank; - std::shared_ptr> bufferHandler; + std::shared_ptr> bufferHandler; std::shared_ptr> loggingHandler; }; @@ -38,6 +38,7 @@ void compareNumericParameter(const std::string& paramString, T expectedValue, } } +// Test: The information stored when calling getBuffer is correct TYPED_TEST(TypedLoggingBufferHandlerTest, GetBufferLogsCorrectly) { auto buffer = this->loggingHandler->getBuffer(100, 1.5); const auto& logs = this->loggingHandler->getLogs(); @@ -58,6 +59,7 @@ TYPED_TEST(TypedLoggingBufferHandlerTest, GetBufferLogsCorrectly) { EXPECT_EQ(entry.rank, this->rank); } +// Test: The information stored when calling freeBuffer is correct TYPED_TEST(TypedLoggingBufferHandlerTest, FreeBufferLogsCorrectly) { auto buffer = this->loggingHandler->getBuffer(100, 1.0); this->loggingHandler->freeBuffer(buffer); @@ -73,6 +75,7 @@ TYPED_TEST(TypedLoggingBufferHandlerTest, FreeBufferLogsCorrectly) { EXPECT_EQ(entry.rank, this->rank); } +// Test: The information stored when calling freeAllBuffers is correct TYPED_TEST(TypedLoggingBufferHandlerTest, FreeAllBuffersLogsCorrectly) { this->loggingHandler->getBuffer(100, 1.0); this->loggingHandler->getBuffer(200, 1.0); @@ -89,6 +92,7 @@ TYPED_TEST(TypedLoggingBufferHandlerTest, FreeAllBuffersLogsCorrectly) { EXPECT_EQ(entry.rank, this->rank); } +// Test: The information stored when calling deleteAllBuffers is correct TYPED_TEST(TypedLoggingBufferHandlerTest, DeleteAllBuffersLogsCorrectly) { this->loggingHandler->getBuffer(100, 1.0); this->loggingHandler->getBuffer(200, 1.0);