Skip to content
This repository has been archived by the owner on Aug 18, 2024. It is now read-only.

Improve performance of MembershipManager #485

Open
claudiu-cristea opened this issue May 2, 2019 · 4 comments
Open

Improve performance of MembershipManager #485

claudiu-cristea opened this issue May 2, 2019 · 4 comments
Labels
Drupal 8 Performance Performance issues or improvements

Comments

@claudiu-cristea
Copy link
Collaborator

claudiu-cristea commented May 2, 2019

In #480 the internal static cache has been replaced by using the @cache.static service. This, now, opens the way to double the static caching with persistent caching (database, Redis, etc).

All we need is to declare a service, such as:

og.membership.memory_cache:
  class: Drupal\Core\Cache\MemoryCache\MemoryCache
og.membership.cache:
  class: Drupal\Core\Cache\BackendChain
  calls:
    - [appendBackend, ['@og.membership.memory_cache']]
    - [appendBackend, ['@cache.data']]
  tags:
    # This ensures cache tag invalidation.
    - { name: cache.bin }

Then we just need to inject this instead of @cache.static and do only some renaming inside MembershipManager.

Even not used in core (but tested), BackendChain allows retrieving items in cascade. You can add as many backends you want. When getting an item from cache, the service will look first in the first backend (which in our case is memory). If there are items not retrieved from there, will fallback to the next defined backend and so on. All subsequent backends cache is tagged with the same tags, so invalidation will also work nice.

@claudiu-cristea
Copy link
Collaborator Author

This still needs module maintainer OK.

@pfrenssen
Copy link
Contributor

I'm very interested in this. I have been looking at the ChainedFastBackend while I was recently working on the caching of the MembershipManager but since there were no real examples in core I didn't set it up.

So this would mean we could persistently cache our current static cache, and still guarantee the integrity through the use of the cache tags and contexts? Basically persisting our cache between requests?

@claudiu-cristea
Copy link
Collaborator Author

claudiu-cristea commented May 6, 2019

(...) I have been looking at the ChainedFastBackend (...)

I fact is \Drupal\Core\Cache\BackendChain.

So this would mean we could persistently cache our current static cache, and still guarantee the integrity through the use of the cache tags and contexts? Basically persisting our cache between requests?

Exactly.

I will provide a PR as soon I as I'll get some oxygen

@pfrenssen
Copy link
Contributor

I did profiling on MembershipManager::loadMemberships() and the actual database query takes only 6ms for a user with 1000+ memberships while retrieving the (cached) memberships at the end (the $this->loadMemberships() call) takes 69ms.

I am not sure if we would gain a lot by caching this, especially since the majority of users will not have 1000+ memberships. It seems like retrieving the cached data would be similarly fast.

I did not check the other methods in the membership manager though. Possibly ::getGroupIds() would benefit the most? I would suggest to do some profiling on this before starting work on this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Drupal 8 Performance Performance issues or improvements
Projects
None yet
Development

No branches or pull requests

3 participants