Move contexts checks async
haeiven opened this issue · 6 comments
Description
Luckperms contexts are checked in the main thread, making the performance poor when used a lot by plugins
Proposed behaviour
Probably make it async? :/
Do you have any profiling data to back this up?
ContextCalculators should be fast and non-blocking - suitable for use inline and on the server thread.
Ah I see - well, ideally DiscordSRV should be caching this value, so the MySQL lookup isn't necessary.
While this would solve the problem, it causes another; another server might change the data in the database, which would lead to the cache being out of date, and adding redis/plugin message for this would be quite the hurdle (this is just one value that's in the database)
however, I don't think the MySQL storage was there at the time.. I think that might've been added since.
It was however, it extends a regular class, making it hard to find
In all fairness, the very first thing that the ContextCalculator interface "specification" mentions is that they ought to be fast and should resolve time-consuming queries and cache their results beforehand
Please see Luck's statement;
I feel partially responsible, since I initially PRed contexts into DiscordSRV - however, I don't think the MySQL storage was there at the time.. I think that might've been added since.
I already did some experimenting with caching the values for a while while players are online, but seems like that wasn't enough - I'll see what I can do on DiscordSRV's side to mitigate this
Also, Merry Christmas :)
After asking to one of DiscordSRV dev:
HaeivenAujourd’hui à 19:35
mh... i don't rly understand why the mysql request isn't done in async?
VankkaAujourd’hui à 19:36
because luckperms is calculating contexts for the user on the main thread
And this is not the only one plugin to have this problem, as I see :/
Ah I see - well, ideally DiscordSRV should be caching this value, so the MySQL lookup isn't necessary.
I feel partially responsible, since I initially PRed contexts into DiscordSRV - however, I don't think the MySQL storage was there at the time.. I think that might've been added since.
Anyway, unfortunately there's not much LP can do about this - contexts have to be calculated whenever permissions are checked, which of course is going to happen sometimes on the main thread.
If you're not using DiscordSRV contexts, then it is probably easiest to just disable the integration in DiscordSRVs config. Otherwise, it's something that is going to need to be fixed at DSRVs end.
And this is not the only one plugin to have this problem, as I see :/
I'm not aware of any others?
In all fairness, the very first thing that the ContextCalculator interface "specification" mentions is that they ought to be fast and should resolve time-consuming queries and cache their results beforehand:
https://github.com/lucko/LuckPerms/blob/8167fbf73f99debb0cd8c1a46412a0176afb88af/api/src/main/java/net/luckperms/api/context/ContextCalculator.java#L33-L43