Bypass expensive access checks

I opened MR 82, implementing the proposed resolution. The code is based on @larowlan's comment #3306434-14: [Config] Warning: Entity view display 'node.article.default': Component 'comment' was disabled because its settings depend on removed dependencies. → .

It might be a good idea to improve this by making the affected types configurable. Then site administrators who know that there are no permissions to manage could completely remove those forms (unset($route_providers['permissions']); in the hook implementation); those who know there are permissions to manage could access them without the slow check.

FWIW, seems wrong to me to add this hack to this module. Lets focus on improving this in core.

Not sure if this slowness got worse with Drupal 10.3.1 but that is what we have noticed. We had to disable admin_toolbar_tools because it was causing 15-20 second page load times for admin users.

You should look at the referenced core issue and test and report if that's working for you.

Marking needs more info because its quite likely this is fixed with the core issue being resolved for D10 and 11.

@todea:

Not sure if this slowness got worse with Drupal 10.3.1 ...

I hope you mean from 10.2.? to 10.3.1, not from 10.3.0 to 10.3.1.

This problem got worse in 10.3 as a side effect of the bug fix 🐛 [regression] Do not bypass route access with 'link to any page' permissions for menu links Fixed . Before that, many of the links in the admin menu were skipping the access check. See this comment: #3306434-66: Fix access checks for bundle permissions to avoid triggering a config validation error → . Also #39 on that issue.

I would love to know how to reproduce your 15-20 second load times, starting with a fresh install of Drupal.

Have you tried the change in this issue and/or the change from 🐛 Fix access checks for bundle permissions to avoid triggering a config validation error Fixed ? That issue is fixed in the 10.3.x branch, so it will be part of the next release for 10.3.

@japerry:

#3306434 was fixed, but 🐛 Don't hide permissions local tasks on bundles when no permissions are defined Needs work is still open. I will be very happy if the first issue fixes this problem, but @Berdir is pretty sure that we need the second.

I am not sure whether your comment was referring to #3306434 or an expectation that #3384600 will soon be fixed.

heh, I'm not sure either. ;) I haven't ran into this issue personally yet, so lets let this bake for a while before having to make any specific hacks in this module.

@benjifisher:
Yes, we went from I believe 10.2.7 straight to 10.3.1

Not sure if I can replicate the 15-20 second load times w/ something like simplytest.me but it was pretty slow. Back in February we upgraded from Drupal 7 to 10 and it has been a bit of a bumpy road so far. We have ~200 enabled modules, ~299k nodes. Some of the permission related modules we use are: override_node_options, view_unpublished, workbench, workbench_access.

At the time, disabling admin_toolbar_tools quickly solved the problem and the difference was night & day.

On our dev server I re-enabled admin_toolbar_tools and I applied patch #5 from 3384600 → and your MR 82 listed above (we needed both). That has fixed the issue on our end, our pages that utilize admin toolbar are back to normal now - much faster.

Thank you to both you and @berdir for working on these various issues.

@todea:

On our dev server I re-enabled admin_toolbar_tools and I applied patch #5 from 3384600 and your MR 82 listed above (we needed both).

That is really surprising. Those two changes should have exactly the same effect. Is it possible that after applying one change, you did not clear caches before testing?

Drupal 10.3.2 was released last week, and it includes the fix for 🐛 Fix access checks for bundle permissions to avoid triggering a config validation error Fixed . That issue does not prevent the slow access check, but it does prevent the log message each time the check is done, which should help. I would really like to know whether it helps a little or a lot.

Ideally, I would like to know how long it takes, after clearing caches, to load an admin page with the admin_toolbar_tools module enabled, using the same database and each of these codebases:

1. Drupal 10.3.1, no patches.
2. Drupal 10.3.2, no patches.
3. Drupal 10.3.2, patch from #3384600.
4. Drupal 10.3.2, patch admin_toolbar_tools from this issue.
5. Drupal 10.3.2, patches from both (3) and (4).

The difference between (1) and (2) will tell us how much #3306434 helps, if at all.

I expect that (3), (4), and (5) will have the same result. If it turns out that you did not clear caches in your earlier test, then I do not really care about (4) and (5).

If I were doing the test, I would use my local development environment (DDEV). That makes it easy to switch the codebase without having to push the changes to the dev server.

We are dealing with noticeable differences here, so we do not need to time things to the millisecond. If you just clear caches, start a timer, reload the page, and stop the timer when the page finishes loading, that should tell us what we need to know.

I spent the past couple of weeks troubleshooting (e.g. database tuning) some extremely slow page loads on admin pages. Today I found this thread and I think it solved my problem.

I'll try to address the questions from @benjifisher in the previous comment although the Drupal versions are different.

I chose a random user profile and recorded the following in Firefox dev tools (network tab, cache disabled). All values are for the first line in the dev tools output -- initiator: document, type: html. There are a lot of other elements (js, css) loading after that but they're all pretty fast:

Rebuild cache before each of these:

1. drupal 10.3.5, admin toolbar 3.5.0 installed and admin_toolbar_tools 3.5.0 installed (Untitled-6.png)
-- waiting 13.07s, receiving 458 ms
2. drupal 10.3.5, admin toolbar 3.5.0 installed and admin_toolbar_tools 3.5.0 uninstalled (Untitled-7.png)
-- waiting 3.20s, receiving 400 ms
3. drupal 10.3.5, admin toolbar 3.5.0 uninstalled and admin_toolbar_tools 3.5.0 uninstalled (Untitled-8.png)
-- waiting 2.96s, receiving 308 ms
4. drupal 10.3.5, admin toolbar 3.5.0 installed and admin_toolbar_tools 3.5.0 installed with patch from core issue #3384600 (MR9500) (Untitled-9.png)
-- waiting 3.58s, receiving 307 ms
5. drupal 10.3.5, admin toolbar 3.5.0 installed and admin_toolbar_tools 3.5.0 installed with patch from this (MR82) (Untitled-10.png)
-- waiting 4.21s, receiving 491 ms
6. drupal 10.3.5, admin toolbar 3.5.0 installed and admin_toolbar_tools 3.5.0 installed with patch from this (MR82) (Untitled-11.png)
-- waiting 4.86s, receiving 528 ms

So ... either patch helps, but the core patch helps slightly more, and both togather help slightly less.

For now I think I'm just going to use the core patch (issue #3384600 MR9500).

@rclemings:

Thanks so much for these data! I have been asking for something like this for a while. Since you did not test with Drupal 10.3.1, we do not know how much difference #3306434 helps (if at all), but that is less important than knowing how much #3384600 helps.

I do not see any difference between (5) and (6). Does (6) include both patches?

Could you add a similar comment to 🐛 Don't hide permissions local tasks on bundles when no permissions are defined Needs work ? If you do not, then I will. I think it is easier to read in tabular format:

All tests were done with Drupal 10.3.5, and admin_toolbar 3.5.0.

Here is the markdown source for that table:

Patch from #3384600 | Patch from #3459723 | `admin_toolbar` enabled | `admin_toolbar_tools` enabled | waiting | receiving
--------------------|---------------------|-------------------------|-------------------------------|--------:|---------:
no                  | no                  | yes                     | yes                           | 13.07s  | 458 ms
no                  | no                  | yes                     | no                            | 03.20s  | 400 ms
no                  | no                  | no                      | no                            | 02.96s  | 308 ms
yes                 | no                  | yes                     | yes                           | 03.98s  | 307 ms
no                  | yes                 | yes                     | yes                           | 04.21s  | 491 ms
yes (?)             | yes                 | yes                     | yes                           | 04.86s  | 528 ms

I think #6 must be with both patches applied, as you surmised. I didn't keep my notes, unfortunately.

Cross-posting to issue 3384600 momentarily.

@rclemings: thanks again!

Nice table, looks like
this patch https://git.drupalcode.org/project/admin_toolbar/-/merge_requests/82.patch is the winner for the performance comparison.
📌 Bypass slow access checks Postponed: needs info

Marking this as RTBC with witnesses indicating that this is not fixed.

This issue should be closed. This is literally doing the same thing as the core issue which has been commited to 11.x and just needs to be backported to 10.x

I agree with #17. I think #16 is mis-reading the table.

In #3384600-68: Don't hide permissions local tasks on bundles when no permissions are defined → , @catch suggests that the last three rows of the table are withing the margin of error. Taking the numbers at face value, the winner is the core patch (#3384600, fourth row), not the patch on this issue (fifth row).

The only reason we might want to apply the fix in this issue is because the core issue will not be in a release until at least 10.4.0.

ah ok, I misunderstood, ok good to know there is a solution in core for D11! Something to look forward to!

I don't think the extra overhead is worth it for the old versions of Drupal. If folks want to patch this module individually, thats cool -- but otherwise just wait for the next version of Drupal to come out.