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

Proposal: State and Cache providers #2238

Open
quinchs opened this issue Apr 8, 2022 · 11 comments · May be fixed by #2251
Open

Proposal: State and Cache providers #2238

quinchs opened this issue Apr 8, 2022 · 11 comments · May be fixed by #2251

Comments

@quinchs
Copy link
Member

quinchs commented Apr 8, 2022

Proposal

Memory usage is a big issue within the library, developers have minimal control on how entities are stored in state. The proposal is to allow developers to fully customize the way Discord.Net stores entities in memory.

Foxbot has the first part of this covered with the IStateProvider allowing users to control how entities are fetched-stored to and from the cache. The state provider allows for downloading entities if they don't exist within the cache for almost all gateway models and events.

The ICacheProvider is something I've been thinking about for way too long. I've finalized my idea on implementing this into Discord.Net.
For each entity that can be cached ex IGuildMember there will be a IGuildMemberModel interface that allows us to have multiple ways to implement converting to and from strong-typed entities to models. We can make the api model Discord.API.Member and make it inherit the IGuildMemberModel interface and take in the interface for constructing a SocketGuildMember or RestGuildMember entity. Developers can create their own models that inherit IGuildMemberModel for example a class that converts itself to a protobuf buffer and pass that to the state provider to use to create an entity.

The term "state provider" and "cache provider" are two separate things, the state provider determines how entities are retrieved and stored while the cache provider stores to and fetches from the cache. Take this flow diagram as some more explanation behind this:

dnet-state-cache-flow

Things to note

The state provider will have to return the interface types of entities ex IGuild, IUser, etc due to the ability to fetch from rest if the behavior is enabled, this will cause a lot of gateway events to have to use interfaces due to this.

@quinchs
Copy link
Member Author

quinchs commented Apr 14, 2022

Design and Progress report

For the past week I've been designing the api and restructuring our entities to abide by it. Currently I've switched over the users (SocketUser, SocketGuildMember, etc) to the new system.

I would like some feedback on some design decisions I've made:

  1. Bring-your-own-model approach
    You as a developer can create your own model classes/structs for each entity type. This allows you to store extra info, control memory behavior, serialization/deserialization rules, add funcs, etc. Each entity has its own entity model interface (ex IUser->IUserModel) that has the minimum required info about that entity to create it.

  2. Sync and async lookups
    The state provider and cache provider should be designed to handle both synchronous and asynchronous lookups of entities, this means all calls will return ValueTask and ValueTask<T>. Writing it like this allows us to keep properties like SocketGuildUser.Guild and DiscordSocketClient.GetUser() as we can preform a synchronous cache lookup.

  3. Lazy loading of entities
    Properties like SocketGuildUser.Guild will now wrap the Lazy<T> struct to allow for lazy loading from cache, this should help with memory as entities are only loaded if they are used.

  4. Small internal cache
    Discord.Net will still contain a non-configurable small internal cache that only stores entities that are currently in scope of you bot. This is to allow for gateway events to update "in-use"/referenced entities that your functions are still using. an example of why this is important is if for example you have a long running command and you're watching the users nickname and waiting for it to change it will get updated by the gateway.
    To help with this all socket entities will be disposable.

Previews

Here's what I have so far for the cache provider and state provider interfaces

ICacheProvider
    public interface ICacheProvider
    {
        #region Users

        ValueTask<IUserModel> GetUserAsync(ulong id, CacheRunMode runmode);
        ValueTask<IEnumerable<IUserModel>> GetUsersAsync(CacheRunMode runmode);
        ValueTask AddOrUpdateUserAsync(IUserModel model, CacheRunMode runmode);
        ValueTask RemoveUserAsync(ulong id, CacheRunMode runmode);

        #endregion

        #region Members

        ValueTask<IMemberModel> GetMemberAsync(ulong id, ulong guildId, CacheRunMode runmode);
        ValueTask<IEnumerable<IMemberModel>> GetMembersAsync(ulong guildId, CacheRunMode runmode);
        ValueTask AddOrUpdateMemberAsync(IMemberModel model, ulong guildId, CacheRunMode runmode);
        ValueTask RemoveMemberAsync(ulong id, ulong guildId, CacheRunMode runmode);

        #endregion

        #region Presence

        ValueTask<IPresenceModel> GetPresenceAsync(ulong userId, CacheRunMode runmode);
        ValueTask AddOrUpdatePresenseAsync(ulong userId, IPresenceModel presense, CacheRunMode runmode);
        ValueTask RemovePresenseAsync(ulong userId, CacheRunMode runmode);

        #endregion
    }
IStateProvider
    public interface IStateProvider
    {
        ValueTask<IPresence> GetPresenceAsync(ulong userId, StateBehavior stateBehavior);
        ValueTask AddOrUpdatePresenseAsync(ulong userId, IPresence presense, StateBehavior stateBehavior);
        ValueTask RemovePresenseAsync(ulong userId, StateBehavior stateBehavior);

        ValueTask<IUser> GetUserAsync(ulong id, StateBehavior stateBehavior, RequestOptions options = null);
        ValueTask<IEnumerable<IUser>> GetUsersAsync(StateBehavior stateBehavior, RequestOptions options = null);
        ValueTask AddOrUpdateUserAsync(IUser user);
        ValueTask RemoveUserAsync(ulong id);

        ValueTask<IGuildUser> GetMemberAsync(ulong guildId, ulong id, StateBehavior stateBehavior, RequestOptions options = null);
        ValueTask<IEnumerable<IGuildUser>> GetMembersAsync(ulong guildId, StateBehavior stateBehavior, RequestOptions options = null);
        ValueTask AddOrUpdateMemberAsync(ulong guildId, IGuildUser user);
        ValueTask RemoveMemberAsync(ulong id);
    }

Defaults defaults defaults!!

The library will include some basic default cache providers to get users started. Here's my ideas so far

  • Redis cache provider
  • MemoryCache cache provider

With the way cache providers work its very simple to make extension packages for different cache providers so we can leave more up to the community.

@budgetdevv
Copy link

a long running command and you're watching the users nickname and waiting for it to change it will get updated by the gateway.

I assume such is only possible with Task.Run() ? Seeing that commands are blocking atm

@quinchs
Copy link
Member Author

quinchs commented Apr 14, 2022

commands are blocking atm

Commands with RunMode.Async aren't

@budgetdevv
Copy link

Yeah since they're task.run under the hood 👀

@Cenngo
Copy link
Collaborator

Cenngo commented Apr 14, 2022

Bring-your-own-model approach
You as a developer can create your own model classes/structs for each entity type. This allows you to store extra info, control memory behavior, serialization/deserialization rules, add funcs, etc.

This is something large bot devs can really benefit from but if I remeber correcty this was something the OG devs had to compromise because of:

Due to the nature of the Discord API, we will oftentimes need to add a property to an entity to support the latest API changes. Discord.Net provides interfaces as a method of consuming entities; and as such, introducing a new field to an entity is technically a breaking change. Major version bumps generally indicate some major change to the library, and as such we are hesitant to bump the major version for every minor addition to the library. To compromise, we have decided that interfaces should be treated as consumable only, and your applications should typically not be implementing interfaces. (For applications where interfaces are implemented, such as in test mocks, we apologize for this inconsistency with SemVer).

so we have that to consider.


Sync and async lookups
The state provider and cache provider should be designed to handle both synchronous and asynchronous lookups of entities, this means all calls will return ValueTask and ValueTask. Writing it like this allows us to keep properties like SocketGuildUser.Guild and DiscordSocketClient.GetUser() as we can preform a synchronous cache lookup.

it makes sense to use ValueTasks if we are going to synchronously short circuit lookup operations often.


Lazy loading of entities
Properties like SocketGuildUser.Guild will now wrap the Lazy struct to allow for lazy loading from cache, this should help with memory as entities are only loaded if they are used.

if we are going to pull the guild reference from the cache wouldnt that defeat the purpose of using Lazy<>. Initializing the guild field of a socket event wouldnt be expensive and should only have a small overhead

@budgetdevv
Copy link

Wouldn't the lazy struct have a larger initialization overhead? Seeing that if we avoid the lazy route, we're just assigning a reference ( I'm assuming you can't really have a socketguilduser without its guild in cache )

@quinchs quinchs linked a pull request Apr 18, 2022 that will close this issue
12 tasks
@quinchs quinchs linked a pull request Apr 18, 2022 that will close this issue
12 tasks
@quinchs
Copy link
Member Author

quinchs commented Apr 24, 2022

Update 2

Cache provider interface v248912412

The entire cache provider interface has changed again, I've decided to implement the feedback I got from quahu and volt about the interface and add a IEntityStore concept. You can take a look at the new interface as well as the store here. This newer system eliviates the issue of having to implement all the crud methods for all the entities in one class :lmao:. You can find the default concurrent cache provider and store here.

All entities are now disposable

Entities can now be disposed to release their reference and data back to the cache, with in-memory caching this isn't really too useful but with stuff like redis or other caching mediums outside of your bots process it can make a big difference. Discord.Net will maintain its own internal list of currently in-use entities being used by your code, whenever an entity falls out of scope or is disposed dnet will release it from the internal list and place it back into the cache. Why have this you might ask? This allows for entities to be update on the fly from the gateway, If you're holding a reference to an entity in your code it will remain up to date by the gateway.

Lazy loading

All referenced entities within other entities are now lazily loaded, This helps remove unnecessary cache lookups to build the same strong typed entities you know and love. We're now exposing a LazyCached<T> type for entity properties that can support both sync and async lookups from the cache. You can see that type here.

Async and Sync mixing

The cache provider should support both async and sync CRUD operations, this behavior is defined by the CacheRunMode control passed into each CRUD operation, This allows you to keep your old sync code working as well as adding async where needed. The client will always use async where available to call the cache provider, the only non-async part is controlled by you as the developer.

Models models models

The system still is a bring-your-own model approach, allowing you (if needed) to create classes that represent entity models. This feature is helpful incase you need to add attributes for serialization or other features/functions to your models. All entities that support caching have a .ToModel<T>() function that will convert it to your specified model type.

@AddressXception
Copy link

Is this still on the roadmap? I'm building something and using this library but running into a few limitations that I need to work around. I left my comments in this discussion here:

#2508

thanks!

@quinchs
Copy link
Member Author

quinchs commented Jan 13, 2023

@AddressXception Hey! I've been a bit out-of-touch with dnet as I've recently gotten a job which requires a lot of my time. The feature in question (and subsequent v4 features) are kind of hard to pull off and require a lot of time I don't have at the moment.

Maybe @Cenngo or @csmir can pick this back up?

If any miracles appear in my schedule I'll look back into this.

@csmir
Copy link
Collaborator

csmir commented Jan 14, 2023

I will take a look. V4 should be a primary focus in the near future

@AddressXception
Copy link

appreciate the updates. in the short term I'm unblocked by just holding onto object state in memory while requests are in-flight. I'm working on a project that is using this library so I'm here to help in any way I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Proposals
Development

Successfully merging a pull request may close this issue.

5 participants