fix: revert to weakrefs for user cache

changelog/858.bugfix.rst

@@ -0,0 +1 @@
+Fix user cache memory leak where unused objects weren't being evicted (provided that :attr:`Intents.members` is enabled).

disnake/state.py

@@ -9,6 +9,7 @@
 import itertools
 import logging
 import os
+import weakref
 from collections import OrderedDict, deque
 from typing import (
     TYPE_CHECKING,
@@ -271,7 +272,6 @@ def __init__(
 
         if not self._intents.members or member_cache_flags._empty:
             self.store_user = self.create_user
-            self.deref_user = self.deref_user_no_intents
 
         self.parsers = parsers = {}
         for attr, func in inspect.getmembers(self):
@@ -284,19 +284,11 @@ def clear(
         self, *, views: bool = True, application_commands: bool = True, modals: bool = True
     ) -> None:
         self.user: ClientUser = MISSING
-        # Originally, this code used WeakValueDictionary to maintain references to the
-        # global user mapping.
-
-        # However, profiling showed that this came with two cons:
-
-        # 1. The __weakref__ slot caused a non-trivial increase in memory
-        # 2. The performance of the mapping caused store_user to be a bottleneck.
-
-        # Since this is undesirable, a mapping is now used instead with stored
-        # references now using a regular dictionary with eviction being done
-        # using __del__. Testing this for memory leaks led to no discernable leaks,
-        # though more testing will have to be done.
-        self._users: Dict[int, User] = {}
+        # NOTE: without weakrefs, these user objects would otherwise be kept in memory indefinitely.
+        # However, using weakrefs here unfortunately has a few drawbacks:
+        # - the weakref slot + object in user objects likely results in a small increase in memory usage
+        # - accesses on `_users` are slower, e.g. `__getitem__` takes ~1us with weakrefs and ~0.2us without
+        self._users: weakref.WeakValueDictionary[int, User] = weakref.WeakValueDictionary()
         self._emojis: Dict[int, Emoji] = {}
         self._stickers: Dict[int, GuildSticker] = {}
         self._guilds: Dict[int, Guild] = {}
@@ -389,18 +381,11 @@ def store_user(self, data: UserPayload) -> User:
             user = User(state=self, data=data)
             if user.discriminator != "0000":
                 self._users[user_id] = user
-                user._stored = True
             return user
 
-    def deref_user(self, user_id: int) -> None:
-        self._users.pop(user_id, None)
-
     def create_user(self, data: UserPayload) -> User:
         return User(state=self, data=data)
 
-    def deref_user_no_intents(self, user_id: int) -> None:
-        return
-
     def get_user(self, id: Optional[int]) -> Optional[User]:
         # the keys of self._users are ints
         return self._users.get(id)  # type: ignore
@@ -735,7 +720,8 @@ def parse_ready(self, data: gateway.ReadyEvent) -> None:
         self._ready_state: asyncio.Queue[Guild] = asyncio.Queue()
         self.clear(views=False, application_commands=False, modals=False)
         self.user = ClientUser(state=self, data=data["user"])
-        self.store_user(data["user"])
+        # self._users is a list of Users, we're setting a ClientUser
+        self._users[self.user.id] = self.user  # type: ignore
 
         if self.application_id is None:
             try:
@@ -1004,11 +990,8 @@ def parse_presence_update(self, data: gateway.PresenceUpdateEvent) -> None:
         self.dispatch("presence_update", old_member, member)
 
     def parse_user_update(self, data: gateway.UserUpdateEvent) -> None:
-        user: ClientUser = self.user
-        user._update(data)
-        ref = self._users.get(user.id)
-        if ref:
-            ref._update(data)
+        if user := self.user:
+            user._update(data)
 
     def parse_invite_create(self, data: gateway.InviteCreateEvent) -> None:
         invite = Invite.from_gateway(state=self, data=data)

disnake/user.py

@@ -425,30 +425,11 @@ class User(BaseUser, disnake.abc.Messageable):
         Specifies if the user is a system user (i.e. represents Discord officially).
     """
 
-    __slots__ = ("_stored",)
-
-    def __init__(
-        self, *, state: ConnectionState, data: Union[UserPayload, PartialUserPayload]
-    ) -> None:
-        super().__init__(state=state, data=data)
-        self._stored: bool = False
+    __slots__ = ("__weakref__",)
 
     def __repr__(self) -> str:
         return f"<User id={self.id} name={self.name!r} discriminator={self.discriminator!r} bot={self.bot}>"
 
-    def __del__(self) -> None:
-        try:
-            if self._stored:
-                self._state.deref_user(self.id)
-        except KeyError:
-            pass
-
-    @classmethod
-    def _copy(cls, user: User) -> Self:
-        self = super()._copy(user)
-        self._stored = False
-        return self
-
     async def _get_channel(self) -> DMChannel:
         ch = await self.create_dm()
         return ch