Skip to content
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

Open
BartArys opened this issue Dec 19, 2020 · 1 comment
Open

Provide full entities on events #147

BartArys opened this issue Dec 19, 2020 · 1 comment

Comments

@BartArys
Copy link
Contributor

Kord's core entities are designed to optimize caching; instead of a Guild keeping a reference to its Channels, it'll keep a reference to the channel IDs, and query the EntitySupplier 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.

@HopeBaron
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants