From ac849b958f1f317f97fea839e87de04373f7d128 Mon Sep 17 00:00:00 2001 From: Elad <18193363+elad335@users.noreply.github.com> Date: Sat, 16 Nov 2024 20:35:23 +0200 Subject: [PATCH] Progress: Dialog: Fixup memory management --- rpcs3/Emu/scoped_progress_dialog.cpp | 119 +++++++++------------------ rpcs3/Emu/system_progress.hpp | 7 +- 2 files changed, 44 insertions(+), 82 deletions(-) diff --git a/rpcs3/Emu/scoped_progress_dialog.cpp b/rpcs3/Emu/scoped_progress_dialog.cpp index a1775d5d18..b7a016fc20 100644 --- a/rpcs3/Emu/scoped_progress_dialog.cpp +++ b/rpcs3/Emu/scoped_progress_dialog.cpp @@ -10,17 +10,36 @@ shared_ptr progress_dialog_string_t::get_string_ptr() const noexcep { text.reset(); + if (old_val.text_count == 0) + { + return text; + } + if (old_val.text_index != umax) { - if (auto ptr = g_progr_text_queue[old_val.text_index].load()) + if (text = g_progr_text_queue[old_val.text_index].load(); !text) { - text = ptr; + // Search for available text (until the time another thread sets index) + const u64 queue_size = g_progr_text_queue.size(); + + for (u64 i = queue_size - 1;; i--) + { + if (i == umax) + { + return text; + } + + if ((text = g_progr_text_queue[i].load())) + { + return text; + } + } } } auto new_val = data.load(); - if (old_val.text_index == new_val.text_index) + if (old_val.update_id == new_val.update_id) { break; } @@ -62,86 +81,36 @@ scoped_progress_dialog::scoped_progress_dialog(std::string text) noexcept } } - usz unmap_index = umax; - g_progr_text.data.atomic_op([&](std::common_type_t& progr) { - unmap_index = progr.text_index; progr.update_id++; progr.text_index = m_text_index; + progr.text_count++; }); - - // Note: unmap_index may be m_text_index if picked up at the destructor - // Which is technically the newest value to be inserted so it serves its logic - if (unmap_index != umax && unmap_index != m_text_index) - { - g_progr_text_queue[unmap_index].reset(); - } } scoped_progress_dialog& scoped_progress_dialog::operator=(std::string text) noexcept { - shared_ptr installed_ptr = make_single_value(std::move(text)); - const shared_ptr c_null_ptr; - - for (usz init_size = g_progr_text_queue.size(), new_text_index = std::max(init_size, 1) - 1;;) - { - if (new_text_index >= init_size) - { - // Search element in new memeory - new_text_index++; - } - else if (new_text_index == 0) - { - // Search element in new memeory - new_text_index = init_size; - } - else - { - new_text_index--; - } - - auto& info = g_progr_text_queue[new_text_index]; - - if (!info && info.compare_and_swap_test(c_null_ptr, installed_ptr)) - { - m_text_index = new_text_index; - break; - } - } - - usz unmap_index = umax; - - g_progr_text.data.atomic_op([&](std::common_type_t& progr) - { - unmap_index = progr.text_index; - progr.update_id++; - progr.text_index = m_text_index; - }); - - // Note: unmap_index may be m_text_index if picked up at the destructor - // Which is technically the newest value to be inserted so it serves its logic - if (unmap_index != umax && unmap_index != m_text_index) - { - g_progr_text_queue[unmap_index].reset(); - } - + // Exchange text atomically + g_progr_text_queue[m_text_index].exchange(make_single_value(std::move(text))); return *this; } scoped_progress_dialog::~scoped_progress_dialog() noexcept { - bool unmap = false; + // Deallocate memory + g_progr_text_queue[m_text_index].reset(); + + auto progr = g_progr_text.data.load(); while (true) { - auto progr = g_progr_text.data.load(); - usz restored_text_index = umax; + u32 restored_text_index = umax; - if (progr.text_index == m_text_index) + if (progr.text_index == m_text_index && !g_progr_text_queue[m_text_index]) { // Search for available text - // Out of scope of atomic to keep atomic_op as clean as possible (for potential future enhacements) + // Out of scope of atomic loop to keep atomic_op as clean as possible (for potential future enhacements) const u64 queue_size = g_progr_text_queue.size(); for (u64 i = queue_size - 1;; i--) @@ -152,11 +121,6 @@ scoped_progress_dialog::~scoped_progress_dialog() noexcept break; } - if (i == m_text_index) - { - continue; - } - if (g_progr_text_queue[i].operator bool()) { restored_text_index = i; @@ -165,7 +129,7 @@ scoped_progress_dialog::~scoped_progress_dialog() noexcept } } - if (!g_progr_text.data.atomic_op([&](std::common_type_t& progr0) + auto [old, ok] = g_progr_text.data.fetch_op([&](std::common_type_t& progr0) { if (progr.update_id != progr0.update_id) { @@ -173,16 +137,14 @@ scoped_progress_dialog::~scoped_progress_dialog() noexcept return false; } - unmap = false; + progr0.text_count--; if (progr0.text_index == m_text_index) { - unmap = true; - if (restored_text_index == umax) { progr0.text_index = umax; - progr0.update_id = 0; + progr0.update_id++; return true; } @@ -191,16 +153,15 @@ scoped_progress_dialog::~scoped_progress_dialog() noexcept } return true; - })) + }); + + progr = old; + + if (!ok) { continue; } break; } - - if (unmap) - { - g_progr_text_queue[m_text_index].reset(); - } } diff --git a/rpcs3/Emu/system_progress.hpp b/rpcs3/Emu/system_progress.hpp index 98fe854875..13ae8d13eb 100644 --- a/rpcs3/Emu/system_progress.hpp +++ b/rpcs3/Emu/system_progress.hpp @@ -12,7 +12,8 @@ struct alignas(16) progress_dialog_string_t struct alignas(16) data_t { usz update_id = 0; - usz text_index = umax; + u32 text_index = umax; + u32 text_count = 0; }; atomic_t data{}; @@ -31,7 +32,7 @@ struct alignas(16) progress_dialog_string_t explicit operator bool() const noexcept { - return get_string_ptr().operator bool(); + return data.load().text_count != 0; } }; @@ -49,7 +50,7 @@ extern atomic_t g_system_progress_stopping; class scoped_progress_dialog final { private: - usz m_text_index = 0; + u32 m_text_index = 0; public: scoped_progress_dialog(std::string text) noexcept;