We have our own admin menu implementation in the `social_admin_menu` which tries to enhance the default setup we have from contrib:
- admin toolbar
- gin toolbar
- drupal's toolbar
By making it more user friendly for our site managers and content managers, which are not administrator / site builder / developer oriented roles.
However there are several performance issues with this.
1. It tries to load the entire `admin` menu, which is mainly focused on site builders, and afterwards revoke access to links they don't have access too
2. It comes with a lot of duplicate links, because in Open Social we tend to try and make the default links less site builder / developer oriented and copy them with a more user friendly title on a better position, which causes duplicates
3. There are quite some additional modules trying to enhance it, for example gin tries to make the menu 4 levels deep, which means all access checks needs to run for all links that are up to 4 levels deep. Which in the case of a big product like Open Social counts for almost 900(!!) links to check.
4. The cache contexts are set to change on different perspectives, this is because of the all the contrib modules we use. For example gin_toolbar needs to cache it per route context for the active menu trail. Which means the cache is not kept across routes, in our case we are sure the cache doesn't need to change across routes so we can implement our own contexts and revoke others.
Similarly with Bootstrap, it adds the is_front context, our toolbar does not need to change on the frontpage versus others.
It's easiest to spot with: https://www.drupal.org/node/3162480
You'll see all the cache context and render time it takes for the admin toolbar to render.
Make sure we control the cache contexts, we know we should be able to cache this for users with the same set of permissions, theme, and language. The only down side right now is that it might hurt the active menu trail, something we don't use in Open Social.
Considering the performance impact we moved forward with it.
Merge: https://git.drupalcode.org/project/socialbase/-/merge_requests/164 so we can release it at the same time.
Add screenshot of the old vs new menu.
Write tests.
Instead of the entire admin menu, we render Open Social's admin menu.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | Screenshot 2023-05-16 at 15.02.47.png | 586.54 KB | ronaldtebrake |
| #11 | Screenshot 2023-05-16 at 15.02.56.png | 630.55 KB | ronaldtebrake |
| #4 | 3349597-performance-issues-with-social_admin_menu-4.patch | 11.11 KB | tbsiqueira |
Comments
Comment #2
ronaldtebrake commentedWork in progress;
https://github.com/goalgorilla/open_social/pull/3355
Comment #3
ronaldtebrake commentedPart one will be tackled here; https://github.com/goalgorilla/open_social/pull/3358
https://github.com/goalgorilla/open_social/pull/3355 will be closed for now as it needs some UX / Design love to make sure the actual menu links we show to our SM and CM are of the quality it needs, which will take some time for a new design iteration before we can do that.
However the cache contexts as fixed in https://github.com/goalgorilla/open_social/pull/3358
are going to already create a great impact
Comment #4
tbsiqueiraComment #5
socialnicheguru commentedHow can I test this?
Does this mean that this will replace admin_toolbar or some of it's submodules?
Comment #6
socialnicheguru commentedComment #7
ronaldtebrake commentedComment #8
ronaldtebrake commentedHi SocialNicheGuru
I've updated the solution section with our chosen way forward.
If you're planning to try it out, I think it's easiest to check here:
https://github.com/goalgorilla/open_social/pull/3358#:~:text=Performance...(cache%20context)
We thought about replacing the admin toolbar or some of it's submodules but the impact there was backwards incompatible and the performance issue was that impactful we decided to first move forward with fixing the cache. This also gives our design team some time to see how we could better create an Open Social admin menu and the impact that would mean for any of the admin menu's we currently have.
Hope it helps!
Comment #9
socialnicheguru commentedOk so I am a little unclear.
Does this save the admin_toolbar:
1) per role for the site
2) per role per page
3) per user per page
I am not getting a boost going from page to page. I thought the admin toolbar loaded all submenus so it shouldn't matter if I click from one page to another. The admin_toolbar should be retrieved from role based cache.
As long as I can get clarity on which one it is, I might be able to use something like http://drupal.org/project/warmer to warm the cache ahead of time
Comment #10
ronaldtebrake commentedSo the idea is that for the toolbar as a render item, this caches it per role for the entire site.
Do note that with using custom modules they might have their own way of influencing the cache.
We've made sure it's working for Open Social and the modules we ship with. For example `gin_toolbar` uses https://git.drupalcode.org/project/gin_toolbar/-/blob/8.x-1.x/gin_toolba... context to vary the cache per page. Just for the active menu trail.
We've verified it locally using https://www.drupal.org/node/3162480 and we get cache hits for the roles and contrib modules we ship with.
Also verified on New Relic & Blackfire it's a big performance bump for our site managers who have the admin toolbar.
We're going to mark this as RTBC and release it very soon.
If you have any insights from cache misses on the toolbar we're happy to hear about that and see if we can help out.
Comment #11
ronaldtebrake commentedattached two screenshots from two pages with a cache render hit for the toolbar
Comment #12
socialnicheguru commentedOk. that makes sense.
I just retired it after clearing cache and a number of other things. It works WELL! so great. So much faster than before
Comment #13
socialnicheguru commentedComment #14
tbsiqueiraComment #15
tbsiqueiraThis is going to be available on the latest 11.8.7 version