-
-
Notifications
You must be signed in to change notification settings - Fork 736
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
Comments
Design and Progress reportFor 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:
PreviewsHere'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
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. |
I assume such is only possible with Task.Run() ? Seeing that commands are blocking atm |
Commands with RunMode.Async aren't |
Yeah since they're task.run under the hood 👀 |
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:
so we have that to consider.
it makes sense to use ValueTasks if we are going to synchronously short circuit lookup operations often.
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 |
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 ) |
Update 2Cache provider interface v248912412The 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 All entities are now disposableEntities 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 loadingAll 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 Async and Sync mixingThe cache provider should support both async and sync CRUD operations, this behavior is defined by the Models models modelsThe 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 |
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: thanks! |
@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. |
I will take a look. V4 should be a primary focus in the near future |
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. |
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 aIGuildMemberModel
interface that allows us to have multiple ways to implement converting to and from strong-typed entities to models. We can make the api modelDiscord.API.Member
and make it inherit theIGuildMemberModel
interface and take in the interface for constructing aSocketGuildMember
orRestGuildMember
entity. Developers can create their own models that inheritIGuildMemberModel
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:
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.The text was updated successfully, but these errors were encountered: