This topic was marked solved by its starter, on August 1st, 2013, 05:52 AM
SMF bug 4763 - menu hook doesn't work properly when caching level >= 2

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
There are two separate issues being discussed but they're heavily related.

Firstly, the items in the menu are cached - including the consequences of calling a hook. Which means on caching level 2, weird things can happen where plugin menu items won't necessarily be displayed (or worse, they'll continue to be displayed even when deactivated)

Secondly, determining the active menu item is potentially inaccurate because of the above.

I'm sort of wondering what the point of caching it is, though. It's very straightforward as it stands, and in fact the caching can screw things up given the gallery unread count.

Perhaps it would simply be better not to cache it at all.
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial

Nao

  • Dadman with a boy
  • Posts: 16,078

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
That sort of defeats the point of caching, because other than Aeva's unread count (which is per user as opposed to the caching which is groups+language), it's all permissions tests which are pretty cheap and there are better places to worry about caching.

Nao

  • Dadman with a boy
  • Posts: 16,078
Is this for the main menu or admin menu (or both?)
If it's admin, then I agree, caching can be disabled to simplify everything...
If it's main menu, no need to bother either, because it's short enough.
If it's non-main and non-admin (profile...), then it gets interesting. I guess.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278

Nao

  • Dadman with a boy
  • Posts: 16,078
Level 2+ isn't recommended by SMF anyway. I understand the logic of having multiple caching levels, but it's not clear what exactly is cached and isn't.

I suppose it'd be good to time the time taken by the admin menu from build time to display time. Just to see how much of a percentage of performance it takes on the page.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Quote
but it's not clear what exactly is cached and isn't.
Nope. But I'm not convinced the main menu is worth caching and even if it is, it still has to be reworked because right now the media count will be wrong.
Quote
I suppose it'd be good to time the time taken by the admin menu from build time to display time. Just to see how much of a percentage of performance it takes on the page.
I'm not that bothered by the ACP performance as I am something that's run nearly every page...

Nao

  • Dadman with a boy
  • Posts: 16,078

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Well... it depends how you define 'dynamic' entries. More than one button is already variable depending on state, e.g. the login and register buttons.

As far as the admin menu goes, simple expansions that only add their own page and nothing more, they're already sort of cached and don't need to load any files, they're in $settings.

I have thought about doing something similar for the main menu too, so that simple menu buttons (nothing that is *too* dynamic) from plugins can be handled much the same, leading to files not being loaded just to make a tiny change to the array.

For more complex cases, caching is probably useless anyway and will convolute things more than it'll help.

Nao

  • Dadman with a boy
  • Posts: 16,078

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278