From 52813cc66247bc40ae205a5cef51ec8262c11790 Mon Sep 17 00:00:00 2001 From: Lukas Burgholzer Date: Thu, 14 Sep 2023 05:13:01 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=B8=20=20Small=20corrections=20for=20s?= =?UTF-8?q?tatistics=20tracking=20(#424)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Another small follow-up to #419 that squashed another couple of quirks in the statistics tracking. ## Checklist: - [x] The pull request only contains commits that are related to it. - [x] I have added appropriate tests and documentation. - [x] I have made sure that all CI jobs on GitHub pass. - [x] The pull request introduces no new warnings and follows the project's style guidelines. --------- Signed-off-by: burgholzer --- include/dd/UniqueTable.hpp | 6 ++--- src/dd/MemoryManager.cpp | 11 ++++----- src/dd/statistics/MemoryManagerStatistics.cpp | 19 +++++++-------- test/dd/test_complex.cpp | 23 ++++++++++--------- 4 files changed, 29 insertions(+), 30 deletions(-) diff --git a/include/dd/UniqueTable.hpp b/include/dd/UniqueTable.hpp index aa6bea257..49ea04ba1 100644 --- a/include/dd/UniqueTable.hpp +++ b/include/dd/UniqueTable.hpp @@ -229,11 +229,11 @@ template class UniqueTable { } std::size_t garbageCollect(bool force = false) { - if ((!force && !possiblyNeedsCollection()) || getNumEntries() == 0) { - return 0; + const std::size_t numEntriesBefore = getNumEntries(); + if ((!force && numEntriesBefore < gcLimit) || numEntriesBefore == 0U) { + return 0U; } - const std::size_t numEntriesBefore = getNumEntries(); std::size_t v = 0U; for (auto& table : tables) { auto& stat = stats[v]; diff --git a/src/dd/MemoryManager.cpp b/src/dd/MemoryManager.cpp index 0cfdc76c4..8f32e3c27 100644 --- a/src/dd/MemoryManager.cpp +++ b/src/dd/MemoryManager.cpp @@ -81,19 +81,18 @@ template void MemoryManager::reset(const bool resizeToTotal) noexcept { available = nullptr; - chunks.erase(chunks.begin() + 1, chunks.end()); - for (auto& entry : chunks[0]) { - entry.ref = 0U; - } + auto numAllocations = stats.numAllocations; + chunks.resize(1U); if (resizeToTotal) { chunks[0].resize(stats.numAllocated); + ++numAllocations; } chunkIt = chunks[0].begin(); chunkEndIt = chunks[0].end(); stats.reset(); - stats.numAllocations = 1U; + stats.numAllocations = numAllocations; stats.numAllocated = chunks[0].size(); } @@ -103,8 +102,6 @@ T* MemoryManager::getEntryFromAvailableList() noexcept { auto* entry = available; available = available->next; stats.trackReusedEntries(); - // Reclaimed entries might have a non-zero reference count - entry->ref = 0; return entry; } diff --git a/src/dd/statistics/MemoryManagerStatistics.cpp b/src/dd/statistics/MemoryManagerStatistics.cpp index 9138c65ac..65ac6242a 100644 --- a/src/dd/statistics/MemoryManagerStatistics.cpp +++ b/src/dd/statistics/MemoryManagerStatistics.cpp @@ -11,12 +11,12 @@ namespace dd { template std::size_t MemoryManagerStatistics::getNumAvailableFromChunks() const noexcept { - return numAllocated - numUsed; + return getTotalNumAvailable() - numAvailableForReuse; } template std::size_t MemoryManagerStatistics::getTotalNumAvailable() const noexcept { - return getNumAvailableFromChunks() + numAvailableForReuse; + return numAllocated - numUsed; } template @@ -75,17 +75,18 @@ template nlohmann::json MemoryManagerStatistics::json() const { } nlohmann::json j = Statistics::json(); - j["num_allocations"] = numAllocations; - j["num_allocated"] = numAllocated; j["memory_allocated_MiB"] = getAllocatedMemoryMiB(); - j["num_used"] = numUsed; j["memory_used_MiB"] = getUsedMemoryMiB(); + j["memory_used_MiB_peak"] = getPeakUsedMemoryMiB(); + j["num_allocated"] = numAllocated; + j["num_allocations"] = numAllocations; j["num_available_for_reuse"] = numAvailableForReuse; - j["total_num_available"] = getTotalNumAvailable(); + j["num_available_for_reuse_peak"] = peakNumAvailableForReuse; + j["num_available_from_chunks"] = getNumAvailableFromChunks(); + j["num_available_total"] = getTotalNumAvailable(); + j["num_used"] = numUsed; + j["num_used_peak"] = peakNumUsed; j["usage_ratio"] = getUsageRatio(); - j["peak_num_used"] = peakNumUsed; - j["peak_num_available_for_reuse"] = peakNumAvailableForReuse; - j["peak_memory_used_MiB"] = getPeakUsedMemoryMiB(); return j; } diff --git a/test/dd/test_complex.cpp b/test/dd/test_complex.cpp index e1467d92e..c372f4d3b 100644 --- a/test/dd/test_complex.cpp +++ b/test/dd/test_complex.cpp @@ -474,34 +474,35 @@ TEST_F(CNTest, MaxRefCountReached) { } TEST_F(CNTest, ComplexTableAllocation) { - auto allocs = mm.getStats().numAllocated; + auto mem = MemoryManager{}; + auto allocs = mem.getStats().numAllocated; std::cout << allocs << "\n"; std::vector nums{allocs}; // get all the numbers that are pre-allocated for (auto i = 0U; i < allocs; ++i) { - nums[i] = mm.get(); + nums[i] = mem.get(); } // trigger new allocation - const auto* num = mm.get(); + const auto* num = mem.get(); ASSERT_NE(num, nullptr); - EXPECT_EQ(mm.getStats().numAllocated, + EXPECT_EQ(mem.getStats().numAllocated, (1. + MemoryManager::GROWTH_FACTOR) * static_cast(allocs)); // clearing the complex table should reduce the allocated size to the original // size - mm.reset(); - EXPECT_EQ(mm.getStats().numAllocated, allocs); + mem.reset(); + EXPECT_EQ(mem.getStats().numAllocated, allocs); - EXPECT_EQ(mm.getStats().numAvailableForReuse, 0U); + EXPECT_EQ(mem.getStats().numAvailableForReuse, 0U); // obtain entry - auto* entry = mm.get(); + auto* entry = mem.get(); // immediately return entry - mm.returnEntry(entry); - EXPECT_EQ(mm.getStats().numAvailableForReuse, 1U); + mem.returnEntry(entry); + EXPECT_EQ(mem.getStats().numAvailableForReuse, 1U); // obtain the same entry again, but this time from the available stack - auto* entry2 = mm.get(); + auto* entry2 = mem.get(); EXPECT_EQ(entry, entry2); }