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

Caching improvement #884

Closed
FedorLap2006 opened this issue Mar 1, 2021 · 3 comments
Closed

Caching improvement #884

FedorLap2006 opened this issue Mar 1, 2021 · 3 comments

Comments

@FedorLap2006
Copy link
Collaborator

FedorLap2006 commented Mar 1, 2021

After finishing slash commands (#856), I planned improving caching. This, for now, is one of major problems in the DiscordGo: for example, let's imagine, you put everything into cache... and at some time... you just run out of memory (yeah... if you didn't know, there is no, at all, cleaner for cache). I want to fix such problems and introduce more flexible and comfortable caching. But first of all, I want to discuss this concept with other people here, so I will know what everyone wants, before starting writing the code.

@FedorLap2006
Copy link
Collaborator Author

FedorLap2006 commented Mar 1, 2021

Also, why I brought this here, because if I will start changing the cache module, many people projects may break. So... you now understand my motivation.

@iopred
Copy link
Collaborator

iopred commented Mar 2, 2021

I think a more complex state is a good idea for an add-on library (like dgrouter is for routing).

State in itself is very basic and since the Intents update, it doesn't have much use internally within DiscordGo (previously it was needed for calculating Permissions, which can now be done with the Message variants).

Cache invalidation is one of the "two hard problems" in Computer Science, and I think the best way to solve this issue is by saying "Set StateEnabled = false and manage it yourself" when it comes to modifications to the Core library.

That said, I think there are bugs in State, and it might warrant a rewrite, but considering it is one of the most used Structs in the codebase, I would definitely caution against any backwards incompatible changes.

@FedorLap2006
Copy link
Collaborator Author

FedorLap2006 commented Mar 2, 2021

I think then I should at least fix the problem with infinite filling of the cache. Because, yeah, disable automatic State is a great idea, but whose code is already relying on the State, this bugs (especially infinite filling) may cause a really big troubles. And about more complex state, I agree, maybe I really should implement it in my own library (it's not public yet tho).

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