Skip to content

Commit

Permalink
Improving memory efficiency by eliminating the vtable in the managed …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
RealTimeChris committed Sep 22, 2023
1 parent f65cdca commit 5893781
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 39 deletions.
56 changes: 55 additions & 1 deletion include/dpp/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,61 @@ 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();
/* 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> 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) {
if constexpr (std::is_same_v<emoji, value_type>) {
delete static_cast<emoji*>(g->first);
}
else if constexpr (std::is_same_v<role, value_type>) {
delete static_cast<role*>(g->first);
}
else if constexpr (std::is_same_v<channel, value_type>) {
delete static_cast<channel*>(g->first);
}
else if constexpr (std::is_same_v<guild, value_type>) {
delete static_cast<guild*>(g->first);
}
else if constexpr (std::is_same_v<user, value_type>) {
delete static_cast<user*>(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();
}
}

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

Expand Down
2 changes: 1 addition & 1 deletion include/dpp/emoji.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions include/dpp/managed.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
32 changes: 0 additions & 32 deletions src/dpp/cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,38 +47,6 @@ uint64_t counter () { \
}


/* 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);
Expand Down
6 changes: 5 additions & 1 deletion src/dpp/discordclient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
}
}
}
Expand Down

0 comments on commit 5893781

Please sign in to comment.