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

@@ -255,11 +255,6 @@ template<class T> 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<class type> * 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<typename value_type> inline void garbage_collection() {
+	time_t now = time(nullptr);
+	bool repeat = false;
+	{
+		std::lock_guard<std::mutex> 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<value_type*>(g->first);
+					deletion_queue.erase(g);
+					repeat = true;
+					break;
+				}
+			}
+		} while (repeat);
+		if (deletion_queue.size() == 0) {
+			deletion_queue = {};
+		}
+	}
+	if constexpr (std::is_same_v<emoji, value_type>) {
+		dpp::get_emoji_cache()->rehash();
+	}
+	else if constexpr (std::is_same_v<role, value_type>) {
+		dpp::get_role_cache()->rehash();
+	}
+	else if constexpr (std::is_same_v<channel, value_type>) {
+		dpp::get_channel_cache()->rehash();
+	}
+	else if constexpr (std::is_same_v<guild, value_type>) {
+		dpp::get_guild_cache()->rehash();
+	}
+	else if constexpr (std::is_same_v<user, value_type>) {
+		dpp::get_user_cache()->rehash();
+	}
+}
+
 } // namespace dpp
 

include/dpp/emoji.h

@@ -100,7 +100,7 @@ class DPP_EXPORT emoji : public managed, public json_interface<emoji>  {
 	/**
 	 * @brief Destroy the emoji object
 	 */
-	~emoji() override = default;
+	~emoji() = default;
 
 	/**
 	 * @brief Copy assignment operator, copies another emoji's data

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

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<std::mutex> 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);

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<emoji>();
+				dpp::garbage_collection<role>();
+				dpp::garbage_collection<channel>();
+				dpp::garbage_collection<guild>();
+				dpp::garbage_collection<user>();
 			}
 		}
 	}

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);