From b97b8b07dfdea11958ed3c1411dc5dd4d71e94ca Mon Sep 17 00:00:00 2001 From: RealTimeChris <40668522+RealTimeChris@users.noreply.github.com> Date: Fri, 22 Sep 2023 02:21:46 -0400 Subject: [PATCH] Improving memory efficiency by eliminating the vtable in the managed class. By making the destructor protected - we can remove the possibility that someone tries to delete one of the derived classes by deleting an instance of the managed class, which should allow for not having a virtual destructor. --- include/dpp/cache.h | 53 +++++++++++++++++++++++++++++++++++---- include/dpp/emoji.h | 2 +- include/dpp/managed.h | 10 +++++--- src/dpp/cache.cpp | 33 ------------------------ src/dpp/discordclient.cpp | 6 ++++- src/unittest/test.cpp | 7 +++--- 6 files changed, 64 insertions(+), 47 deletions(-) diff --git a/include/dpp/cache.h b/include/dpp/cache.h index 5895af576d..bf6b73ac6d 100644 --- a/include/dpp/cache.h +++ b/include/dpp/cache.h @@ -255,11 +255,6 @@ template class cache { }; -/** Run garbage collection across all caches removing deleted items - * that have been deleted over 60 seconds ago. - */ -void DPP_EXPORT garbage_collection(); - #define cache_decl(type, setter, getter, counter) /** Find an object in the cache by id. @return type* Pointer to the object or nullptr when it's not found */ DPP_EXPORT class type * setter (snowflake id); DPP_EXPORT cache * getter (); /** Get the amount of cached type objects. */ DPP_EXPORT uint64_t counter (); /* Declare major caches */ @@ -269,5 +264,53 @@ cache_decl(role, find_role, get_role_cache, get_role_count); cache_decl(channel, find_channel, get_channel_cache, get_channel_count); cache_decl(emoji, find_emoji, get_emoji_cache, get_emoji_count); +/** Run garbage collection across all caches removing deleted items + * that have been deleted over 60 seconds ago. + */ + /* Because other threads and systems may run for a short while after an event is received, we don't immediately + * delete pointers when objects are replaced. We put them into a queue, and periodically delete pointers in the + * queue. This also rehashes unordered_maps to ensure they free their memory. + */ + /* Because other threads and systems may run for a short while after an event is received, we don't immediately + * delete pointers when objects are replaced. We put them into a queue, and periodically delete pointers in the + * queue. This also rehashes unordered_maps to ensure they free their memory. + */ +template inline void garbage_collection() { + time_t now = time(nullptr); + bool repeat = false; + { + std::lock_guard delete_lock(deletion_mutex); + do { + repeat = false; + for (auto g = deletion_queue.begin(); g != deletion_queue.end(); ++g) { + if (now > g->second + 60) { + delete static_cast(g->first); + deletion_queue.erase(g); + repeat = true; + break; + } + } + } while (repeat); + if (deletion_queue.size() == 0) { + deletion_queue = {}; + } + } + if constexpr (std::is_same_v) { + dpp::get_emoji_cache()->rehash(); + } + else if constexpr (std::is_same_v) { + dpp::get_role_cache()->rehash(); + } + else if constexpr (std::is_same_v) { + dpp::get_channel_cache()->rehash(); + } + else if constexpr (std::is_same_v) { + dpp::get_guild_cache()->rehash(); + } + else if constexpr (std::is_same_v) { + dpp::get_user_cache()->rehash(); + } +} + } // namespace dpp diff --git a/include/dpp/emoji.h b/include/dpp/emoji.h index ab95736595..e427f41d7d 100644 --- a/include/dpp/emoji.h +++ b/include/dpp/emoji.h @@ -100,7 +100,7 @@ class DPP_EXPORT emoji : public managed, public json_interface { /** * @brief Destroy the emoji object */ - ~emoji() override = default; + ~emoji() = default; /** * @brief Copy assignment operator, copies another emoji's data diff --git a/include/dpp/managed.h b/include/dpp/managed.h index 5e79953517..6baf8670f5 100644 --- a/include/dpp/managed.h +++ b/include/dpp/managed.h @@ -42,10 +42,6 @@ namespace dpp { * @param nid ID to set */ managed(const snowflake nid = 0); - /** - * @brief Destroy the managed object - */ - virtual ~managed() = default; /** * @brief Get the creation time of this object according to Discord. @@ -72,6 +68,12 @@ namespace dpp { * @return false objects are the same id */ bool operator!=(const managed& other) const noexcept; + + protected: + /** + * @brief Destroy the managed object + */ + ~managed() = default; }; } // namespace dpp diff --git a/src/dpp/cache.cpp b/src/dpp/cache.cpp index 715b829dd8..57472049b5 100644 --- a/src/dpp/cache.cpp +++ b/src/dpp/cache.cpp @@ -46,39 +46,6 @@ uint64_t counter () { \ return ( cache_name ? cache_name ->count() : 0 ); \ } - -/* Because other threads and systems may run for a short while after an event is received, we don't immediately - * delete pointers when objects are replaced. We put them into a queue, and periodically delete pointers in the - * queue. This also rehashes unordered_maps to ensure they free their memory. - */ -void garbage_collection() { - time_t now = time(nullptr); - bool repeat = false; - { - std::lock_guard delete_lock(deletion_mutex); - do { - repeat = false; - for (auto g = deletion_queue.begin(); g != deletion_queue.end(); ++g) { - if (now > g->second + 60) { - delete g->first; - deletion_queue.erase(g); - repeat = true; - break; - } - } - } while (repeat); - if (deletion_queue.size() == 0) { - deletion_queue = {}; - } - } - dpp::get_user_cache()->rehash(); - dpp::get_channel_cache()->rehash(); - dpp::get_guild_cache()->rehash(); - dpp::get_role_cache()->rehash(); - dpp::get_emoji_cache()->rehash(); -} - - cache_helper(user, user_cache, find_user, get_user_cache, get_user_count); cache_helper(channel, channel_cache, find_channel, get_channel_cache, get_channel_count); cache_helper(role, role_cache, find_role, get_role_cache, get_role_count); diff --git a/src/dpp/discordclient.cpp b/src/dpp/discordclient.cpp index 96f9f52cd4..5a596b82b4 100644 --- a/src/dpp/discordclient.cpp +++ b/src/dpp/discordclient.cpp @@ -505,7 +505,11 @@ void discord_client::one_second_timer() creator->tick_timers(); if ((time(nullptr) % 60) == 0) { - dpp::garbage_collection(); + dpp::garbage_collection(); + dpp::garbage_collection(); + dpp::garbage_collection(); + dpp::garbage_collection(); + dpp::garbage_collection(); } } } diff --git a/src/unittest/test.cpp b/src/unittest/test.cpp index f3adb662f9..4d66e33f74 100644 --- a/src/unittest/test.cpp +++ b/src/unittest/test.cpp @@ -628,8 +628,9 @@ Markdown lol \\|\\|spoiler\\|\\| \\~\\~strikethrough\\~\\~ \\`small \\*code\\* b { set_test(TS, false); - dpp::managed m(189759562910400512); - set_test(TS, ((uint64_t) m.get_creation_time()) == 1465312605); + dpp::user m{}; + m.id = 189759562910400512; + set_test(TS, ((uint64_t)m.get_creation_time()) == 1465312605); } { @@ -1349,7 +1350,7 @@ Markdown lol \\|\\|spoiler\\|\\| \\~\\~strikethrough\\~\\~ \\`small \\*code\\* b bool message_edit_tested = false; bot.on_message_update([&](const dpp::message_update_t &event) { - if (event.msg.author == bot.me.id) { + if (event.msg.author.id == bot.me.id) { if (event.msg.content == "test edit" && !message_edit_tested) { message_edit_tested = true; set_test(EDITEVENT, true);