-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide full entities on events #147
Comments
However, this would apply to data we know that is not going to change (or are less likely to), or would we transform the cache to have computed properties that would query the cached entity each time we want it? Taking into consideration the frequent hits of the cache in a typical scenario, I don't think that's the way we are planning to solve it. We can apply this to what we think is a critical entity (e.g: message and author). |
Kord's core entities are designed to optimize caching; instead of a
Guild
keeping a reference to itsChannels
, it'll keep a reference to the channel IDs, and query theEntitySupplier
to fetch them. This approach makes it easy for us to keep the cache up to date; every entity is only once in the cache for modifications, insertions and removals.This does come with some issues however: Discord's gateway events often do provide a 'full' entity (e.g. Guild creates will contain full channels instead of core's channel IDs, as well as full members, roles, etc). For users with cache this means extra queries for data that was originally present. For users running without cache this means that data is gone completely and they have to resort to retrieval via REST, which is a major inefficiency.
An attempt to mitigate this can be seen in
Message
objects, which have a reference to a full user object for their authors instead of delegating retrieval to the cache. Since authors are such an important field in the entity, we decided not to separate it from the message itself, even though this does 'corrupt' the cache.To mitigate this, we should start by providing 'full' entities in events whenever possible. These new entities should be incorporated into the current hierarchy (Behavior -> cache optimized Entity -> Full Entity) as much as possible. While this does mean a fair amount of extra classes, I believe this to be a worthwhile change for both cache and non-cache use cases, which should translate in some performance gains.
The text was updated successfully, but these errors were encountered: