From 3e2a5c274455de63bd632932c921f0f235e29b26 Mon Sep 17 00:00:00 2001 From: antoniofilipovic Date: Wed, 27 Sep 2023 14:14:35 +0200 Subject: [PATCH] clean up memory control --- src/memory/memory_control.cpp | 315 +++++----------------------------- src/memory/memory_control.hpp | 74 -------- src/query/interpreter.cpp | 3 - 3 files changed, 44 insertions(+), 348 deletions(-) diff --git a/src/memory/memory_control.cpp b/src/memory/memory_control.cpp index 929449aab..a9c0da6f1 100644 --- a/src/memory/memory_control.cpp +++ b/src/memory/memory_control.cpp @@ -15,6 +15,7 @@ #include #include #include +#include "spdlog/spdlog.h" #include "utils/logging.hpp" #include "utils/memory_tracker.hpp" #include "utils/readable_size.hpp" @@ -33,18 +34,15 @@ namespace memgraph::memory { // NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define STRINGIFY(x) STRINGIFY_HELPER(x) -static std::vector original_hooks_vec; - extent_hooks_t *old_hooks = nullptr; -extent_alloc_t *old_alloc = nullptr; - +/* +This is for tracking per query limit +*/ std::vector arena_allocations{}; std::vector arena_upper_limit{}; std::vector tracking_arenas{}; -ExtentHooksStats extent_hook_stats; - int GetArenaForThread() { #if USE_JEMALLOC unsigned thread_arena{0}; @@ -59,173 +57,15 @@ int GetArenaForThread() { } void TrackMemoryForThread(int arena_id, size_t size) { +#if USE_JEMALLOC tracking_arenas[arena_id] = true; arena_upper_limit[arena_id] = arena_allocations[arena_id] + size; -} - -void PrintJemallocInternalStats() { - bool config_stats{false}; - size_t size_of_config_stats = sizeof(config_stats); - - int err = mallctl("config.stats", &config_stats, &size_of_config_stats, nullptr, 0); - - if (err) { - std::cout << "can't get config.stats" << std::endl; - } - std::cout << "CONFIG:STATS: " << std::boolalpha << config_stats << std::endl; - - if (!config_stats) { - return; - } - - { - size_t mapped{0}; - size_t size_of_mapped = sizeof(mapped); - int err = mallctl("stats.mapped", &mapped, &size_of_mapped, nullptr, 0); - - if (err) { - std::cout << "can't get stats.mapped" << std::endl; - } - - std::cout << "STATS:MAPPED: " << utils::GetReadableSize(mapped) << std::endl; - } - - { - size_t active{0}; - size_t size_of_active = sizeof(active); - int err = mallctl("stats.active", &active, &size_of_active, nullptr, 0); - - if (err) { - std::cout << "can't get stats.active" << std::endl; - } - - std::cout << "STATS:ACTIVE: " << utils::GetReadableSize(active) << std::endl; - } - - { - size_t metadata{0}; - size_t size_of_metadata = sizeof(metadata); - int err = mallctl("stats.metadata", &metadata, &size_of_metadata, nullptr, 0); - - if (err) { - std::cout << "can't get stats.metadata" << std::endl; - } - - std::cout << "STATS:METADATA: " << utils::GetReadableSize(metadata) << std::endl; - } - - { - size_t metadata_thp{0}; - size_t size_of_metadata_thp = sizeof(metadata_thp); - int err = mallctl("stats.metadata", &metadata_thp, &size_of_metadata_thp, nullptr, 0); - - if (err) { - std::cout << "can't get stats.metadata_thp" << std::endl; - } - - std::cout << "STATS:metadata_thp: " << utils::GetReadableSize(metadata_thp) << std::endl; - } - - { - size_t resident{0}; - size_t size_of_resident = sizeof(resident); - int err = mallctl("stats.resident", &resident, &size_of_resident, nullptr, 0); - - if (err) { - std::cout << "can't get stats.resident" << std::endl; - } - - std::cout << "STATS:resident: " << utils::GetReadableSize(resident) << std::endl; - } - - { - size_t retained{0}; - size_t size_of_retained = sizeof(retained); - int err = mallctl("stats.retained", &retained, &size_of_retained, nullptr, 0); - - if (err) { - std::cout << "can't get stats.retained" << std::endl; - } - - std::cout << "STATS:retained: " << utils::GetReadableSize(retained) << std::endl; - } - - { - size_t allocated{0}; - size_t size_of_allocated = sizeof(allocated); - int err = mallctl("stats.allocated", &allocated, &size_of_allocated, nullptr, 0); - - if (err) { - std::cout << "can't get stats.allocated" << std::endl; - } - - std::cout << "STATS:allocated: " << utils::GetReadableSize(allocated) << std::endl; - } -} - -void PrintStats() { - /* - - std::cout << "[TOTAL] RAM:" << utils::GetReadableSize(utils::total_memory_tracker.Amount()) - << ", VIRT: " << utils::GetReadableSize(utils::total_memory_tracker.AmountVirt()); - - auto alloc_commited = extent_hook_stats.alloc.commited.load(std::memory_order_relaxed); - auto alloc_uncommited = extent_hook_stats.alloc.uncommited.load(std::memory_order_relaxed); - - auto dalloc_commited = extent_hook_stats.dalloc.commited.load(std::memory_order_relaxed); - auto dalloc_uncommited = extent_hook_stats.dalloc.uncommited.load(std::memory_order_relaxed); - - std::cout << "[ALLOC] commited: " << alloc_commited << ", " - << "uncommited: " << alloc_uncommited << ", " - << "[DALLOC] commited: " << dalloc_commited << ", " - << "uncommited: " << dalloc_uncommited << std::endl; - - auto destroy_commited = extent_hook_stats.destroy.commited.load(std::memory_order_relaxed); - auto destroy_uncommited = extent_hook_stats.destroy.uncommited.load(std::memory_order_relaxed); - - if (destroy_commited || destroy_uncommited) { - std::cout << "[DESTROY] commited: " << destroy_commited << "uncommited: " << destroy_uncommited << std::endl; - } - - auto purge_forced = extent_hook_stats.purge_forced.counter.load(std::memory_order_relaxed); - auto purge_lazy = extent_hook_stats.purge_lazy.counter.load(std::memory_order_relaxed); - if (purge_forced || purge_lazy) { - std::cout << "[PURGE] forced: " << purge_forced << ", lazy " << purge_lazy << std::endl; - } - - auto commit_cnt = extent_hook_stats.commit.counter.load(std::memory_order_relaxed); - auto decommit_cnt = extent_hook_stats.decommit.counter.load(std::memory_order_relaxed); - - if (commit_cnt || decommit_cnt) { - std::cout << "COMMIT: " << commit_cnt << ", DECOMMIT: " << decommit_cnt << std::endl; - } - - auto split_commited = extent_hook_stats.split.commited.load(std::memory_order_relaxed); - auto split_uncommited = extent_hook_stats.split.uncommited.load(std::memory_order_relaxed); - if (split_commited || split_uncommited) { - std::cout << "[SPLIT] commited: " - << ", uncommited: " << split_uncommited << std::endl; - } - - auto merge_commited = extent_hook_stats.merge.commited.load(std::memory_order_relaxed); - auto merge_uncommited = extent_hook_stats.merge.uncommited.load(std::memory_order_relaxed); - - if (merge_commited || merge_uncommited) { - std::cout << "[merge]commited: " << merge_commited << ", uncommited: " << merge_uncommited << std::endl; - } - */ +#endif } void *my_alloc(extent_hooks_t *extent_hooks, void *new_addr, size_t size, size_t alignment, bool *zero, bool *commit, unsigned arena_ind) { - // dangerous assert, useful for testing - MG_ASSERT(size % 4096 == 0, "Alloc size not multiple of page size"); - - // TODO: THIS CAN ACTUALLY BE REMOVED, only use pointer to old hooks, that is it. - // You don't have hooks per arena code - MG_ASSERT(original_hooks_vec[arena_ind] && original_hooks_vec[arena_ind]->alloc); - - // This needs to be before, to trow exception in case of too big alloc + // This needs to be before, to throw exception in case of too big alloc if (*commit) { arena_allocations[arena_ind] += size; if (tracking_arenas[arena_ind]) { @@ -242,7 +82,7 @@ void *my_alloc(extent_hooks_t *extent_hooks, void *new_addr, size_t size, size_t memgraph::utils::total_memory_tracker.AllocVirt(static_cast(size)); } - auto *ptr = original_hooks_vec[arena_ind]->alloc(extent_hooks, new_addr, size, alignment, zero, commit, arena_ind); + auto *ptr = old_hooks->alloc(extent_hooks, new_addr, size, alignment, zero, commit, arena_ind); if (ptr == nullptr) { if (*commit) { arena_allocations[arena_ind] -= size; @@ -253,90 +93,46 @@ void *my_alloc(extent_hooks_t *extent_hooks, void *new_addr, size_t size, size_t return ptr; } - if (*commit) { - ////spdlog::trace(fmt::format("[ALLOC][RAM] memory pages: {} ,equals to: {}", size / 4096UL, - /// utils::GetReadableSize(size))); - - extent_hook_stats.alloc.commited.fetch_add(1, std::memory_order_relaxed); - - } else { - ////spdlog::trace(fmt::format("[ALLOC][VIRT] memory pages: {} ,equals to: {}", size / 4096UL, - /// utils::GetReadableSize(size))); - extent_hook_stats.alloc.uncommited.fetch_add(1, std::memory_order_relaxed); - } - - PrintStats(); - return ptr; } static bool my_dalloc(extent_hooks_t *extent_hooks, void *addr, size_t size, bool committed, unsigned arena_ind) { - // dangerous assert, useful for testing - MG_ASSERT(size % 4096 == 0, "Dalloc size not multiple of page size!"); - - // MG_ASSERT(original_hooks_vec[arena_ind] && original_hooks_vec[arena_ind]->dalloc); auto err = old_hooks->dalloc(extent_hooks, addr, size, committed, arena_ind); if (err) { - extent_hook_stats.dalloc.error.fetch_add(1); return err; } if (committed) { memgraph::utils::total_memory_tracker.Free(static_cast(size)); arena_allocations[arena_ind] -= size; - extent_hook_stats.dalloc.commited.fetch_add(1, std::memory_order_relaxed); - // spdlog::trace(fmt::format("[DALLOC][RAM] memory pages: {} ,equals to: {}", size / 4096UL, - // utils::GetReadableSize(size))); + } else { memgraph::utils::total_memory_tracker.FreeVirt(static_cast(size)); - extent_hook_stats.dalloc.uncommited.fetch_add(1, std::memory_order_relaxed); } - PrintStats(); - return false; } static void my_destroy(extent_hooks_t *extent_hooks, void *addr, size_t size, bool committed, unsigned arena_ind) { - MG_ASSERT(size % 4096 == 0, "Destroy size not multiple of page size!"); - if (committed) { - // spdlog::trace(fmt::format("[DESTROY][RAM] memory pages: {}, size: {} ", size / 4096UL, - // utils::GetReadableSize(size))); - memgraph::utils::total_memory_tracker.Free(static_cast(size)); arena_allocations[arena_ind] -= size; - extent_hook_stats.destroy.commited.fetch_add(1, std::memory_order_relaxed); } else { - // spdlog::trace(fmt::format("[DESTROY][VIRT] memory pages: {}, size: {} ", size / 4096UL, - // utils::GetReadableSize(size))); memgraph::utils::total_memory_tracker.FreeVirt(static_cast(size)); - extent_hook_stats.destroy.uncommited.fetch_add(1, std::memory_order_relaxed); } - PrintStats(); - - MG_ASSERT(original_hooks_vec[arena_ind] && original_hooks_vec[arena_ind]->alloc); old_hooks->destroy(extent_hooks, addr, size, committed, arena_ind); } static bool my_commit(extent_hooks_t *extent_hooks, void *addr, size_t size, size_t offset, size_t length, unsigned arena_ind) { - MG_ASSERT(length % 4096 == 0, "Commit not multiple of page size"); - - // TODO change to use old hooks - MG_ASSERT(original_hooks_vec[arena_ind] && original_hooks_vec[arena_ind]->commit); - auto err = original_hooks_vec[arena_ind]->commit(extent_hooks, addr, size, offset, length, arena_ind); + auto err = old_hooks->commit(extent_hooks, addr, size, offset, length, arena_ind); if (err) { return err; } - extent_hook_stats.commit.counter.fetch_add(1, std::memory_order_relaxed); - - // spdlog::trace(fmt::format("[COMMIT][RAM] memory pages: {}, size: {} ", length / 4096UL, - // utils::GetReadableSize(length))); memgraph::utils::total_memory_tracker.FreeVirt(static_cast(length)); memgraph::utils::total_memory_tracker.Alloc(static_cast(length)); @@ -345,35 +141,27 @@ static bool my_commit(extent_hooks_t *extent_hooks, void *addr, size_t size, siz static bool my_decommit(extent_hooks_t *extent_hooks, void *addr, size_t size, size_t offset, size_t length, unsigned arena_ind) { - MG_ASSERT(original_hooks_vec[arena_ind] && original_hooks_vec[arena_ind]->alloc); + MG_ASSERT(old_hooks && old_hooks->decommit); auto err = old_hooks->decommit(extent_hooks, addr, size, offset, length, arena_ind); - extent_hook_stats.decommit.counter.fetch_add(1, std::memory_order_relaxed); if (err) { return err; } memgraph::utils::total_memory_tracker.AllocVirt(static_cast(length)); memgraph::utils::total_memory_tracker.Free(static_cast(length)); - // spdlog::trace(fmt::format("[DECOMMIT][RAM] memory pages: {}, size: {} ", length / 4096UL, - // utils::GetReadableSize(length))); - // TODO: check is this correct behavior return false; } static bool my_purge_forced(extent_hooks_t *extent_hooks, void *addr, size_t size, size_t offset, size_t length, unsigned arena_ind) { - extent_hook_stats.purge_forced.counter.fetch_add(1, std::memory_order_relaxed); + MG_ASSERT(old_hooks && old_hooks->purge_forced); + auto err = old_hooks->purge_forced(extent_hooks, addr, size, offset, length, arena_ind); - MG_ASSERT(original_hooks_vec[arena_ind] && original_hooks_vec[arena_ind]->purge_forced); - auto err = original_hooks_vec[arena_ind]->purge_forced(extent_hooks, addr, size, offset, length, arena_ind); - - // if (err) { - // return err; - // } - // spdlog::trace(fmt::format("[PURGE F][RAM] memory pages: {}, size: {} ", length / 4096UL, - // utils::GetReadableSize(length))); + if (err) { + return err; + } memgraph::utils::total_memory_tracker.Free(static_cast(length)); return false; @@ -383,41 +171,23 @@ static bool my_purge_lazy(extent_hooks_t *extent_hooks, void *addr, size_t size, unsigned arena_ind) { // If memory is purged lazily, it will not be cleaned immediatelly if we are not using MADVISE_DONTNEED (muzzy=0 and // decay=0) - - extent_hook_stats.purge_lazy.counter.fetch_add(1, std::memory_order_relaxed); - - // spdlog::trace(fmt::format("[PURGE L][RAM] memory pages: {}, size: {} ", length / 4096UL, - // utils::GetReadableSize(length))); - - MG_ASSERT(original_hooks_vec[arena_ind] && original_hooks_vec[arena_ind]->purge_lazy); - return original_hooks_vec[arena_ind]->purge_lazy(extent_hooks, addr, size, offset, length, arena_ind); + MG_ASSERT(old_hooks && old_hooks->purge_lazy); + return old_hooks->purge_lazy(extent_hooks, addr, size, offset, length, arena_ind); } static bool my_split(extent_hooks_t *extent_hooks, void *addr, size_t size, size_t size_a, size_t size_b, bool committed, unsigned arena_ind) { - if (committed) { - extent_hook_stats.split.commited.fetch_add(1, std::memory_order_relaxed); - } else { - extent_hook_stats.split.uncommited.fetch_add(1, std::memory_order_relaxed); - } - - MG_ASSERT(original_hooks_vec[arena_ind] && original_hooks_vec[arena_ind]->split); - return original_hooks_vec[arena_ind]->split(extent_hooks, addr, size, size_a, size_b, committed, arena_ind); + MG_ASSERT(old_hooks && old_hooks->split); + return old_hooks->split(extent_hooks, addr, size, size_a, size_b, committed, arena_ind); } static bool my_merge(extent_hooks_t *extent_hooks, void *addr_a, size_t size_a, void *addr_b, size_t size_b, bool committed, unsigned arena_ind) { - if (committed) { - extent_hook_stats.merge.commited.fetch_add(1, std::memory_order_relaxed); - } else { - extent_hook_stats.merge.uncommited.fetch_add(1, std::memory_order_relaxed); - } - - MG_ASSERT(original_hooks_vec[arena_ind] && original_hooks_vec[arena_ind]->merge); - return original_hooks_vec[arena_ind]->merge(extent_hooks, addr_a, size_a, addr_b, size_b, committed, arena_ind); + MG_ASSERT(old_hooks && old_hooks->merge); + return old_hooks->merge(extent_hooks, addr_a, size_a, addr_b, size_b, committed, arena_ind); } -static extent_hooks_t custom_hooks = { +static constexpr extent_hooks_t custom_hooks = { .alloc = &my_alloc, .dalloc = &my_dalloc, .destroy = &my_destroy, @@ -429,7 +199,7 @@ static extent_hooks_t custom_hooks = { .merge = &my_merge, }; -static extent_hooks_t *new_hooks = &custom_hooks; +static const extent_hooks_t *new_hooks = &custom_hooks; // TODO this can be designed if we fail setting hooks to rollback to classic jemalloc tracker void SetHooks() { @@ -439,42 +209,37 @@ void SetHooks() { uint64_t sz{sizeof(allocated)}; sz = sizeof(unsigned); - unsigned narenas{0}; - int err = mallctl("opt.narenas", (void *)&narenas, &sz, nullptr, 0); + unsigned n_arenas{0}; + int err = mallctl("opt.narenas", (void *)&n_arenas, &sz, nullptr, 0); if (err) { return; } - std::cout << narenas << " : n arenas" << std::endl; + spdlog::trace("n areanas {}", n_arenas); if (nullptr != old_hooks) { return; } - // get original hooks and update alloc - original_hooks_vec.reserve(narenas + 5); - - for (int i = 0; i < narenas; i++) { + for (int i = 0; i < n_arenas; i++) { arena_allocations.emplace_back(0); arena_upper_limit.emplace_back(0); tracking_arenas.emplace_back(0); std::string func_name = "arena." + std::to_string(i) + ".extent_hooks"; size_t hooks_len = sizeof(old_hooks); - // int err = mallctlRead(func_name.c_str(), &old_hooks); + int err = mallctl(func_name.c_str(), &old_hooks, &hooks_len, nullptr, 0); if (err) { LOG_FATAL("Error getting hooks for jemalloc arena {}", i); } - original_hooks_vec.emplace_back(old_hooks); // Due to the way jemalloc works, we need first to set their hooks // which will trigger creating arena, then we can set our custom hook wrappers err = mallctl(func_name.c_str(), nullptr, nullptr, &old_hooks, sizeof(old_hooks)); - // mallctlWrite(func_name.c_str(), &old_hooks) if (err) { LOG_FATAL("Error setting jemalloc hooks for jemalloc arena {}", i); @@ -487,6 +252,17 @@ void SetHooks() { } } + MG_ASSERT(old_hooks); + MG_ASSERT(old_hooks->alloc); + MG_ASSERT(old_hooks->dalloc); + MG_ASSERT(old_hooks->destroy); + MG_ASSERT(old_hooks->commit); + MG_ASSERT(old_hooks->decommit); + MG_ASSERT(old_hooks->purge_forced); + MG_ASSERT(old_hooks->purge_lazy); + MG_ASSERT(old_hooks->split); + MG_ASSERT(old_hooks->merge); + #endif } @@ -498,16 +274,16 @@ void UnSetHooks() { uint64_t sz{sizeof(allocated)}; sz = sizeof(unsigned); - unsigned narenas{0}; - int err = mallctl("opt.narenas", (void *)&narenas, &sz, nullptr, 0); + unsigned n_arenas{0}; + int err = mallctl("opt.narenas", (void *)&n_arenas, &sz, nullptr, 0); if (err) { return; } - std::cout << narenas << " : n arenas" << std::endl; + spdlog::trace("n areanas {}", n_arenas); - for (int i = 0; i < narenas; i++) { + for (int i = 0; i < n_arenas; i++) { std::string func_name = "arena." + std::to_string(i) + ".extent_hooks"; err = mallctl(func_name.c_str(), nullptr, nullptr, &old_hooks, sizeof(old_hooks)); @@ -524,9 +300,6 @@ void PurgeUnusedMemory() { #if USE_JEMALLOC mallctl("arena." STRINGIFY(MALLCTL_ARENAS_ALL) ".purge", nullptr, nullptr, nullptr, 0); #endif - - std::cout << "PURGE CALLED" << std::endl; - PrintStats(); } #undef STRINGIFY diff --git a/src/memory/memory_control.hpp b/src/memory/memory_control.hpp index 69e2a0ade..da93c1f53 100644 --- a/src/memory/memory_control.hpp +++ b/src/memory/memory_control.hpp @@ -15,29 +15,9 @@ #include "utils/logging.hpp" namespace memgraph::memory { -template -int mallctlHelper(const char *cmd, T *out, T *in) { - size_t out_len = sizeof(T); - int err = mallctl(cmd, out, out ? &out_len : nullptr, in, in ? sizeof(T) : 0); - MG_ASSERT(err != 0 || out_len == sizeof(T)); - - return err; -} - -template -int mallctlRead(const char *cmd, T *out) { - return mallctlHelper(cmd, out, static_cast(nullptr)); -} - -template -int mallctlWrite(const char *cmd, T in) { - return mallctlHelper(cmd, static_cast(nullptr), &in); -} - void PurgeUnusedMemory(); void SetHooks(); void UnSetHooks(); -void PrintStats(); int GetArenaForThread(); void TrackMemoryForThread(int arena_ind, size_t size); void SetGlobalLimit(size_t size); @@ -46,58 +26,4 @@ inline std::atomic allocated_memory{0}; inline std::atomic virtual_allocated_memory{0}; inline size_t global_limit{0}; -struct ExtentHooksStats { - struct Alloc { - std::atomic commited{0}; - std::atomic uncommited{0}; - }; - - struct Dalloc { - std::atomic commited{0}; - std::atomic uncommited{0}; - std::atomic error{0}; - }; - - struct Destroy { - std::atomic commited{0}; - std::atomic uncommited{0}; - }; - - struct PurgeForced { - std::atomic counter{0}; - }; - - struct PurgeLazy { - std::atomic counter{0}; - }; - - struct Merge { - std::atomic commited{0}; - std::atomic uncommited{0}; - }; - - struct Split { - std::atomic commited{0}; - std::atomic uncommited{0}; - }; - - struct Commit { - std::atomic counter{0}; - }; - - struct Decommit { - std::atomic counter{0}; - }; - - Alloc alloc; - Dalloc dalloc; - Destroy destroy; - PurgeForced purge_forced; - PurgeLazy purge_lazy; - Merge merge; - Split split; - Commit commit; - Decommit decommit; -}; - } // namespace memgraph::memory diff --git a/src/query/interpreter.cpp b/src/query/interpreter.cpp index 090604dff..911a1644c 100644 --- a/src/query/interpreter.cpp +++ b/src/query/interpreter.cpp @@ -1304,8 +1304,6 @@ std::optional PullPlan::Pull(AnyStream *strea return std::nullopt; } - memgraph::memory::PrintStats(); - summary->insert_or_assign("plan_execution_time", execution_time_.count()); memgraph::metrics::Measure(memgraph::metrics::QueryExecutionLatency_us, std::chrono::duration_cast(execution_time_).count()); @@ -2978,7 +2976,6 @@ PreparedQuery PrepareInfoQuery(ParsedQuery parsed_query, bool in_explicit_transa action = action_on_complete; pull_plan = std::make_shared(std::move(results)); } - memgraph::memory::PrintStats(); if (pull_plan->Pull(stream, n)) { return action; }