- Issue created by @berdir
- 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
So couple things for other people reading this:
- Cache contexts are supposed to be fast, so that we can load variable things without having to run too much code to figure out the cache ID in VariationCache::createCacheIdFast()
- If your cache context needs to load something (e.g. user roles), that needs to be cached (e.g. entity cache) so your next return value can be calculated quicker
- Trying to make convertTokenToKeys run fewer requests to the cache contexts is, as pointed out in the IS, dangerous territory as the return value may have changed over the course of the request
With that out of the way, I need to point out that this is no longer a Group-specific thing, but a core thing. We now have Access Policy API and, just like Group, it uses a persistent and memory variation cache to store your calculated permissions. So if your permissions are highly variable, we have a bunch of cache contexts to check whenever a permission check is ran. As far as that goes, I would definitely focus on making sure cache contexts are fast.
For the specific case of only having a single cache context (I'm profiling with anonymous user, it's user.roles), running optimizeTokens() seems pointless. It will never be able to optimize a single context away, so we could add an early return?
Agreed, we could make optimizeTokens return early if the count of the array is smaller than 2.
A static cache in convertTokensToKeys? but we'd need to understand if and when context values can change during a request.
Very dangerous territory as explained above.
Using VariationCache as a static cache like that is obviously fairly expensive
Yeah, but it's required for access policies to work. I think the cost-benefit is warranted here given how much is now possible with the access policy API.
group specifically always collects 3 different kinds of things in \Drupal\group\Access\GroupPermissionChecker::hasPermissionInGroup() but will always only use 1-2 of those and never all 3.
Yeah, fully agree we can optimize that in Group.
- 🇳🇿New Zealand quietone
Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.