diff --git a/CHANGELOG.md b/CHANGELOG.md index 41d64cee4..c680564fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,13 @@ Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to ### Fixed +## [1.0.1] - 2019-09-04 + +### Fixed + +- Fixed a bug in DynamicPool where memory could be leaked when allocating a new + block using the "minimum size" for an allocation smaller than the block. + ## [1.0.0] - 2019-07-12 ### Added diff --git a/CMakeLists.txt b/CMakeLists.txt index 421b61155..dcd27df51 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,7 +11,7 @@ cmake_policy(SET CMP0057 NEW) project(Umpire LANGUAGES CXX C - VERSION 1.0.0) + VERSION 1.0.1) set(UMPIRE_VERSION_RC "") diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index ecb052292..ac750afbb 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,3 +1,8 @@ +# v1.0.1 + +- Fixed a bug in DynamicPool where memory could be leaked when allocating a new + block using the "minimum size" for an allocation smaller than the block. + # v1.0.0 - Umpire is MPI-aware (outputs rank information to logs and replays) when diff --git a/docs/sphinx/conf.py b/docs/sphinx/conf.py index 1aae57f89..44341aa4b 100644 --- a/docs/sphinx/conf.py +++ b/docs/sphinx/conf.py @@ -98,7 +98,7 @@ # The short X.Y version. version = u'1.0' # The full version, including alpha/beta/rc tags. -release = u'1.0.0' +release = u'1.0.1' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/docs/sphinx/conf.py.in b/docs/sphinx/conf.py.in index b9dfe8e29..beb67fbfe 100644 --- a/docs/sphinx/conf.py.in +++ b/docs/sphinx/conf.py.in @@ -96,9 +96,9 @@ author = u'David Beckingsale' # built documents. # # The short X.Y version. -version = u'0.3' +version = u'1.0' # The full version, including alpha/beta/rc tags. -release = u'0.3.3' +release = u'1.0.1' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/scripts/make_release_tarball.sh b/scripts/make_release_tarball.sh index 69d1e5bd1..1ef785b88 100755 --- a/scripts/make_release_tarball.sh +++ b/scripts/make_release_tarball.sh @@ -7,7 +7,7 @@ ############################################################################## TAR_CMD=gtar -VERSION=1.0.0 +VERSION=1.0.1 git archive --prefix=umpire-${VERSION}/ -o umpire-${VERSION}.tar HEAD 2> /dev/null diff --git a/src/umpire/strategy/DynamicPool.cpp b/src/umpire/strategy/DynamicPool.cpp index 864141128..4df6cf16b 100644 --- a/src/umpire/strategy/DynamicPool.cpp +++ b/src/umpire/strategy/DynamicPool.cpp @@ -33,17 +33,17 @@ DynamicPool::DynamicPool(const std::string& name, const int align_bytes) noexcept : AllocationStrategy(name, id), m_allocator{allocator.getAllocationStrategy()}, - m_min_alloc_bytes{min_alloc_bytes}, + m_min_alloc_bytes{round_up(min_alloc_bytes, align_bytes)}, m_align_bytes{align_bytes}, m_coalesce_heuristic{coalesce_heuristic}, m_used_map{}, m_free_map{}, m_curr_bytes{0}, - m_actual_bytes{initial_alloc_bytes}, + m_actual_bytes{round_up(initial_alloc_bytes, align_bytes)}, m_highwatermark{0} { - insertFree(m_allocator->allocate(initial_alloc_bytes), - initial_alloc_bytes, true, initial_alloc_bytes); + const std::size_t bytes{round_up(initial_alloc_bytes, align_bytes)}; + insertFree(m_allocator->allocate(bytes), bytes, true, bytes); } DynamicPool::~DynamicPool() @@ -180,10 +180,9 @@ void* DynamicPool::allocate(std::size_t bytes) m_curr_bytes += rounded_bytes; - const int64_t left_bytes{static_cast( - free_size - rounded_bytes)}; + const std::size_t left_bytes{free_size - rounded_bytes}; - if (left_bytes > m_align_bytes) { + if (left_bytes > 0) { insertFree(static_cast(ptr) + rounded_bytes, left_bytes, false, whole_bytes); } @@ -197,11 +196,10 @@ void* DynamicPool::allocate(std::size_t bytes) m_actual_bytes += alloc_bytes; m_curr_bytes += rounded_bytes; - const int64_t left_bytes{static_cast( - m_min_alloc_bytes - rounded_bytes)}; + const std::size_t left_bytes{alloc_bytes - rounded_bytes}; // Add free - if (left_bytes > m_align_bytes) + if (left_bytes > 0) insertFree(static_cast(ptr) + rounded_bytes, left_bytes, false, alloc_bytes); } diff --git a/tests/integration/strategy_tests.cpp b/tests/integration/strategy_tests.cpp index 68bb0255f..d077fdb2b 100644 --- a/tests/integration/strategy_tests.cpp +++ b/tests/integration/strategy_tests.cpp @@ -389,11 +389,43 @@ TEST(ReleaseTest, Works) void* ptr_two = alloc.allocate(1024); alloc.deallocate(ptr_two); + ASSERT_EQ(alloc.getCurrentSize(), 62); EXPECT_NO_THROW(alloc.release()); + ASSERT_LE(alloc.getActualSize(), 64); + alloc.deallocate(ptr_one); + ASSERT_EQ(alloc.getCurrentSize(), 0); + + EXPECT_NO_THROW(alloc.release()); + + ASSERT_EQ(alloc.getCurrentSize(), 0); + ASSERT_LE(alloc.getActualSize(), 0); } +TEST(ReleaseTest, MissingBlocks) +{ + auto& rm = umpire::ResourceManager::getInstance(); + + auto allocator = rm.makeAllocator( + "host_dyn_pool_for_release_2", rm.getAllocator("HOST"), 128, 64); + + void* data_one = allocator.allocate(128); + void* data_two = allocator.allocate(44); + + allocator.deallocate(data_one); + allocator.deallocate(data_two); + + ASSERT_EQ(allocator.getCurrentSize(), 0); + ASSERT_GE(allocator.getActualSize(), 0); + + allocator.release(); + + ASSERT_EQ(allocator.getCurrentSize(), 0); + ASSERT_EQ(allocator.getActualSize(), 0); +} + + TEST(DynamicPool, coalesce) { auto& rm = umpire::ResourceManager::getInstance(); diff --git a/tools/util/ReplayInterpreter.cpp b/tools/util/ReplayInterpreter.cpp index 80505841b..509a3fcd9 100644 --- a/tools/util/ReplayInterpreter.cpp +++ b/tools/util/ReplayInterpreter.cpp @@ -113,12 +113,21 @@ void ReplayInterpreter::buildOperations(void) || m_json["payload"]["minor"] != UMPIRE_VERSION_MINOR || m_json["payload"]["patch"] != UMPIRE_VERSION_PATCH ) { - REPLAY_ERROR("Warning, version mismatch:\n" + REPLAY_WARNING("Warning, version mismatch:\n" << " Tool version: " << UMPIRE_VERSION_MAJOR << "." << UMPIRE_VERSION_MINOR << "." << UMPIRE_VERSION_PATCH << std::endl << " Log version: " << m_json["payload"]["major"] << "." << m_json["payload"]["minor"] << "." << m_json["payload"]["patch"]); + + if (m_json["payload"]["major"] != UMPIRE_VERSION_MAJOR) { + REPLAY_ERROR("Warning, major version mismatch:\n" + << " Tool version: " << UMPIRE_VERSION_MAJOR << "." << UMPIRE_VERSION_MINOR << "." << UMPIRE_VERSION_PATCH << std::endl + << " Log version: " + << m_json["payload"]["major"] << "." + << m_json["payload"]["minor"] << "." + << m_json["payload"]["patch"]); + } } } else { @@ -178,12 +187,21 @@ int ReplayInterpreter::getSymbolicOperation( std::string& raw_line, std::string& || m_json["payload"]["minor"] != UMPIRE_VERSION_MINOR || m_json["payload"]["patch"] != UMPIRE_VERSION_PATCH ) { - REPLAY_ERROR("Warning, version mismatch:\n" + REPLAY_WARNING("Warning, version mismatch:\n" << " Tool version: " << UMPIRE_VERSION_MAJOR << "." << UMPIRE_VERSION_MINOR << "." << UMPIRE_VERSION_PATCH << std::endl << " Log version: " << m_json["payload"]["major"] << "." << m_json["payload"]["minor"] << "." << m_json["payload"]["patch"]); + + if (m_json["payload"]["major"] != UMPIRE_VERSION_MAJOR) { + REPLAY_ERROR("Warning, major version mismatch:\n" + << " Tool version: " << UMPIRE_VERSION_MAJOR << "." << UMPIRE_VERSION_MINOR << "." << UMPIRE_VERSION_PATCH << std::endl + << " Log version: " + << m_json["payload"]["major"] << "." + << m_json["payload"]["minor"] << "." + << m_json["payload"]["patch"]); + } } } else { diff --git a/tools/util/ReplayMacros.hpp b/tools/util/ReplayMacros.hpp index 16f9ce2f1..fbf7cc819 100644 --- a/tools/util/ReplayMacros.hpp +++ b/tools/util/ReplayMacros.hpp @@ -10,6 +10,12 @@ #include #include +#define REPLAY_WARNING( msg ) \ +{ \ + std::cout << std::string(__FILE__) << " " << __LINE__ << " " << __func__ \ + << " " << msg << std::endl; \ +} + #define REPLAY_ERROR( msg ) \ { \ std::cerr << std::string(__FILE__) << " " << __LINE__ << " " << __func__ \