Joomla slow with > 3000 menu items, each with > 300 parameters

[fatica]

Hi all, I realize this may be an edge case, but we use Joomla for a SaaS product where our customers can essentially build Joomla menu items (of a single type, view and layout) against our custom component. The options for this menu item type are vast (over 300) and include some heavy objects like HTML document fragments, short lists and so on.

As we've grown and the number of menu items have increased, the entire site has slowed significantly over time. A regular page load for a com_content item got up to 4,000 ms. This value stayed consistent regardless of the component or template involved in the page load. I decided pull up KCachegrind and dive into the issue more scientifically as sinking more $$ into hardware was not scaling.

I found that, regardless of the page, the JMenu class was loading each and every menu item into memory and parsing the parameters into a JRegistry object. The critical element starts in /libraries/cms/menu/site.php, JMenu::load();

I found that if I modified the query in JMenu::load() to load only those menu items of concern to that user, the page load times plummeted to < 250ms. The query fix I used is particular to our unique use of Joomla, and is not appropriate for Joomla core. I'm not familiar enough with JMenu to understand why it loads every menu item regardless of the menus involved on a given request. I'm certainly confused as to why it also parses each menu item's parameters during this process. The combination of the query and the subsequent params parsing into a JRegistry object is where the performance hit occurs.

I can't comment on how to address the issue, since I'm not familiar with JMenu's use throughout the site. However, if there where some way to reduce the scope of the JMenu::load() query for large sites with lots of menu items, I imagine a performance gain could be had. The performance gain for us was huge, and a real face-palm moment.

This issue is in the latest version of Joomla, 3.6.4, but I know this has been around since prior to 2.5.

P.s. There is no "Performance" category, so I used "Libraries"

Steps to reproduce the issue

1. Create an arbitrary custom component (or choose an existing component)
2. Create an Menu item for that component with > 300 options of various types and populate with settings.
3. Create > 3,000 menu items of the type in Test #2.

Expected result

I would expect that performance of the entire site would not decrease proportionally to the number of menu items created above.

Actual result

Page load times for the entire site increase proportionally to the number of menu items created above. Reaches un-usability at about 6,000 menu items.

System information (as much as possible)

A dev box is typically configured as follows, and this mirrors (roughly) what's in production. We've had this same issue across multiple configurations, Web servers, OS's etc.

PHP Built On Linux coffee 3.13.0-61-generic #100-Ubuntu SMP Wed Jul 29 11:22:15 UTC 2015 i686
Database Version 5.6.33-0ubuntu0.14.04.1
Database Collation latin1_swedish_ci
Database Connection Collation utf8mb4_general_ci
PHP Version 5.5.9-1ubuntu4.19
Web Server Apache/2.4.7 (Ubuntu)
WebServer to PHP Interface apache2handler
Joomla! Version Joomla! 3.6.4 Stable [ Noether ] 21-October-2016 16:33 GMT
Joomla! Platform Version Joomla Platform 13.1.0 Stable [ Curiosity ] 24-Apr-2013 00:00 GMT
User Agent Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.99 Safari/537.36
Safe Mode Off
Open basedir None
Display Errors On
Short Open Tags Off
File Uploads On
Magic Quotes Off
Register Globals Off
Output Buffering On
Session Save Path /var/lib/php5
Session Auto Start 0
XML Enabled Yes
Zlib Enabled Yes
Native ZIP Enabled Yes
Disabled Functions pcntl_alarm,pcntl_fork,pcntl_waitpid,pcntl_wait,pcntl_wifexited,pcntl_wifstopped,pcntl_wifsignaled,pcntl_wexitstatus,pcntl_wtermsig,pcntl_wstopsig,pcntl_signal,pcntl_signal_dispatch,pcntl_get_last_error,pcntl_strerror,pcntl_sigprocmask,pcntl_sigwaitinfo,pcntl_sigtimedwait,pcntl_exec,pcntl_getpriority,pcntl_setpriority,
Multibyte String (mbstring) Enabled Yes
Iconv Available Yes
Mcrypt Enabled No
Maximum Input Variables 5000
PHP Version 5.5.9-1ubuntu4.19

Additional comments

Thank you!

[ggppdk]

Yes it has been like this a long time,
->params are made into a JRegistry, for all menu items, by parsing their params string

---

Changing behaviour of this not to pre-parse the parameters of ALL menu items,
will break all code that uses menu item parameters,
which assumes that params are already a JRegistry object

Still, i think performance can improved for some (a few) views / tasks without a B/C break
by moving the parsing of parameters into

• getItem() and getItems(), getParams($id), getMenu() and to ... ? other ?

and in them, checking if ->params is not already an object and parsing it

But in the cases that getMenu() is called then ALL items are returned, thus the parameters parsing will be forced on all of them and thus benefit is gone !!!

So after moving the parsing of parameters, all code that uses getMenu(), needs to be checked to find
if they really need to call getMenu() or they do not need it

---

About fixing performance fully, it can be with a B/C break:

• do not parse at all and instead make a requirement that an API method is called if parameters are needed (i say add an API method, since it is not good to let the caller do parsing and then not store the JRegistry object, for re-using it later)

e.g.

$item = $menu->getItem(41);
$menu->loadParams($item);  // new API method

[ggppdk]

And trying to store all these JRegistry objects into cache
will consume so much time by unserialize, that the performance benefit will be mostly cancelled, right ?

[mbabker]

It's not so much the Registry object that is expensive to cache, it's the data store when you push class objects into it. So if the store is only data (stdClass objects or arrays) then the difference is really between a serialized string versus JSON encoded (which the Registry supports too).

On Thu, Dec 1, 2016 at 5:19 PM Georgios Papadakis ***@***.***> wrote: And trying to store all these JRegistry objects into cache will consume so much time, because of serialize / unserialize, that the performance benefit will be cancelled, right ? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#13054 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAWfoS__0GqnP-b-E3KpOI7ZtgvvA13xks5rD1YEgaJpZM4LB8Dr> .

-- - Michael Please pardon any errors, this message was sent from my iPhone.

[Bakual]

Don't we need all menu items loaded for the sef routing? Not sure if we need the params there but I think we do need them.

[mbabker]

JRouterSite makes use of a JMenuSite object to handle routing, and it does have a place where it loops over the entire item array. It also makes no use of the menu item parameters.

While the "lazy loading" manner of converting parameters to a Registry object might help performance some (and it should be a bit better with 3.6.2 I believe it was where the Registry was updated to bypass some of the more expensive operations when setting the first bit of data into a newly instantiated object), since we don't map everything to a JMenuItem type object where we could handle converting the params property to a Registry when it's actually converted, as rightfully pointed out it would be a B/C break to do something in 3.x where this auto-create behavior is bypassed so that's not an option right now.

If we map each loaded item to a new JMenuItem object which basically corresponds to the menu table's data structure (so now we have a semi-typehinted object, quickest comparison would be to JCategoryNode which does have a getParams() method and has lazy loading of the params registry) and we can put a magic getter in there that handles converting params on first access from the JSON string it's stored in the database as to a Registry object, it might help matters some. The question then becomes is it considered a B/C break to change JMenu::$_items from an array of stdClass objects to an array of JMenuItem objects.

[ggppdk]

@mbabker
I have modified /libraries/cms/menu/menu.php

to test your thought / suggestion (adding a new class JMenuItem with a magic getter for params)
and it seems to working without errors (and i have quite a few 3rd-party extensions installed in this testing site)

(i renamed original string to "params_string" so that ->params will not be set and magic getter will be called)

JMenuItem IS-A stdClass, so there should not be problems with it

[csthomas]

What about use similar method/variables as in https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/user/user.php

[mbabker]

It's the same concept. Except JUser is "doing it wrong", it instantiates a Registry instance for the params object in the constructor then binds the JSON string when real data is loaded. So that approach doesn't fix anything; it's the same problem as exists now with JMenu::load().

[ssnobben]

Good that you find and investigate this fatica!

"Reaches un-usability at about 6,000 menu items." Scary performance scenario if peoples sites grow into bigger sites : ) :(

I also see some lower performance for a 5 language multilanguage Joomla 3.6.4 site with options of various types with about 560+ menus and wonder how to solve this. My impression of impact is that It’s also getting slower also when adding more menus with a lot of different language modules.

Hope Joomla devs can investigate, prioritize and solve a solution for this performance issues for Joomla (3.6.5?) bcs it’s also something important for Joomlas larger projects and sites.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13054.}

[mbabker]

#13073 should be good for testing on this.

[fatica]

Hi all, First of all, thank you so much for your attention. I certainly did not expect the immediate and qualified response I received. Kudos! We tested this patch: https://github.com/joomla/joomla-cms/pull/13073/files And did not have the performance gains we were hoping for. I may not be testing this correctly, but with a test set of about 3,000 menu items I still see a significant delay in page loads. There's about a 1ms per menu item delay. It's possible that with the new caching this could improve, but I have not tested that yet. Michael Fatica Principal Fatica Consulting L.L.C. 303-325-5912 www.fatica.net

…

On Sat, Dec 3, 2016 at 12:06 PM, zero-24 ***@***.***> wrote: Closed #13054 <#13054>. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#13054 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAzCP4LzNPS372jxOgF6qYsFdmsw824Iks5rEb2mgaJpZM4LB8Dr> .

[mbabker]

It won't completely fix your issue, but it should help. As was pointed out on the pull request, the performance gains of lazy converting the parameters are negated by rendering menu items since that process will parse the parameters.

The caching should help since that'll minimize the number of times the database gets queried to pull the data, presumably it should be faster to load a serialized array from a caching engine than executing that database query.

It's definitely not as great as things could be, but should still be helpful in some cases.

[csthomas]

I suspect the cache is the second problem.
I propose to test again with cache off.

[ggppdk]

@fatica

Quoting mbabker:

As was pointed out on the pull request, the performance gains of lazy converting the parameters are negated by rendering menu items since that process will parse the parameters.

Consider if you really need to show all menus in the current page,

if you do show all menu items or most of them, then you will see no benefits

[metalocator]

FYI, we still have this same issue in 3.8. The root issue is the query (now in SiteMenu.php) that pulls every menu item. We still are using a home-grown core patch to solve the issue.

@ggppdk We don't show all menu items on the current page. However, Joomla loads them all regardless.

[mbabker]

Joomla has to load all items. The menu module in part reads the parameters for all loaded menu items to determine if an item should be shown, that can't be applied through a SQL WHERE clause without either extracting it from the params field or making a MySQL 5.7+ requirement with the use of its JSON field type (and that too would have to be portable to other supported database engines).

The result of the query loading items is going to be cached when you have the caching layer enabled. That is going to help a fair bit. But with how closely the menu and routing systems are tied together, you aren't going to find an efficient way to make that particular query load less data that doesn't result in extra queries for individual menu items (so you're trading off an eager load query for a potential N + 1 scenario).

Not knowing anything of your home grown patch, there is no way to evaluate if that could be used in core to help with performance, or point out any potential issues with its implementation that could be causing unexpected behavior.

[csthomas]

@metalocator Can you put one of your json string here?

[ggppdk]

@metalocator

FYI, we still have this same issue in 3.8. The root issue is the query (now in SiteMenu.php) that pulls every menu item. We still are using a home-grown core patch to solve the issue.

No, what you mention about SQL query is a performance degrade

• than is much smaller than the performance issue that was fixed by Create a JMenuItem object to represent menu items in the JMenu API #13073

#13073 fixed the parameter creation for all menus items regards if it they are used

i don't say that it is not an issue, it is, i just say it is smaller than what was fixed by #13073

[metalocator]

Thanks again for the quick responses. You guys are awesome.

Joomla has to load all items.

This seems like a pretty big scaling issue for us.

I wonder if the query could be changed to avoid the ordering. That seems to be the most expensive attribute of the query.

Our patch (which is super-specific to us) is below. The real takeaway is that we introduce a few WHERE conditions that reduce the universe of consideration down to only those menu items that pertain to the SPA we're showing, or the Admin interface, or CLI, or the XMLRPC API. I doubt those would be portable to any other implementation.

Our main issue with this is that most visits to our SPA are to a single page with a unique Itemid. It establishes a cache sure, but the next visit to a different URL appears to establish a new cache with the exact same startup overhead since (?) cache is keyed by URL.

Removing this patch immediately changes page load times from 100ms to 6000ms and up, which in times of heavy load would make the entire she-bang fall to pieces ;-)

Our app is a single-page application, so our issue is mainly that of the initial load. Each menu item (of which we have about 6000) is a unique configuration of said SPA.

Adding new customers, and new SPAs is slowly making the entire application slower. So we need the below hack, or we need to move to a different framework altogether in the long term.

@csthomas example attached, though after @mbabker's lazy loading patch, I don't think the parameter loading is our real issue, it seems to be just the query.

Thank you again!
SiteMenu-diff.txt
params-9187.txt

[csthomas]

Instead of one backtick ` use ``` to quote multiline code.

[mbabker]

That particular query's cache shouldn't be generating a new entry, otherwise there's another bug to be checked (unless something else is colliding with it somehow), its cache key is just an MD5 hash of the SiteMenu FQCN (so it should basically always be d17e8e13b99b4d9bf23a7d0b83a466c3). That said, there are other parts of the system which use request data to generate the cache key, but this specific instance should most assuredly stay consistent. Note there is a difference in behavior with the page cache plugin, which IIRC is one of the most heavily request reliant cache pieces of the system, and the global conservative/progressive caching config.

I wonder if the query could be changed to avoid the ordering. That seems to be the most expensive attribute of the query.

If it can be done in PHP without giving a major performance tradeoff then it might be possible/practical.

[csthomas]

@metalocator

Please go to you phpmyadmin and execute below query.
How many seconds it takes? Can you compare it to joomla load?
If time of execution query is similar to joomla load then the problem is query and we will have to add better INDEX in mysql.

SELECT m.id, m.menutype, m.title, m.alias, m.note, m.path AS route, m.link, m.type, m.level, m.language,`m`.`browserNav`, m.access, m.params, m.home, m.img, m.template_style_id, m.component_id, m.parent_id,e.element as component

  FROM `#__menu` AS m

  LEFT JOIN `#__extensions` AS e 
  ON m.component_id = e.extension_id

  WHERE m.published = 1 
  AND m.parent_id > 0 
  AND m.client_id = 0

  ORDER BY m.lft

[metalocator]

2.9626 seconds for the query. 4000 rows.

We used KCacheGrind to identify this query as the most expensive aspect of the page load.

[csthomas]

In phpmyadmin execute this query ALTER TABLE `#__menu` ADD INDEX `idx_client_lft` (`client_id`, `lft`); and then check previous query again.

[metalocator]

No change, I think it actually ran slower with that index. I ran the previous on production (and I can't add that index there). In dev, the query went from 1s to 2.2s after adding the index. Both attempts using SQL_NO_CACHE.

[csthomas]

Did you test the original query or your modified?
Please go to phpmyadin and execute

EXPLAIN SELECT m.id, m.menutype, m.title, m.alias, m.note, m.path AS route, m.link, m.type, m.level, m.language,`m`.`browserNav`, m.access, m.params, m.home, m.img, m.template_style_id, m.component_id, m.parent_id,e.element as component

  FROM `#__menu` AS m

  LEFT JOIN `#__extensions` AS e 
  ON m.component_id = e.extension_id

  WHERE m.published = 1 
  AND m.parent_id > 0 
  AND m.client_id = 0

  ORDER BY m.lft

[csthomas]

Besides that, you may have a problem with the poorly configured mysql server.

If you have a read access to mysql configuration then check value for innodb_buffer_pool_size
For big joomla database you need to increase this value to ex 512MB, 1GB or even 2.5GB.

[metalocator]

The explain result is below. This was from the exact query you posted, unmodifed. Total time is 2.260 seconds. That time remained constant as I increased innodb_buffer_pool_size from the default to 1024M, and 2048M. This test server is a brand new Ryzen Threadripper, 32GB of RAM etc.

I'm not sure that this would be solved by indexes. The core issue seems to be a problem with the size of the response. Remember that our jos_menu.params content is quite large.

One thing I did notice is that if I remove the params field from the query, the speed jumps to 0.025 sec. That speed remains if I remove the index recommended by @csthomas. Is the params field still necessary in this query given the new lazy load approach? Removing it from the query breaks menu icons (and likely other things). Seems like we might have a solution if it wasn't required.

Again, thank you!

id	select_type	table	partitions	type	possible_keys	key	key_len	ref	rows	filtered	Extra
1	SIMPLE	m	\N	range	idx_client_id_parent_id_alias_language,idx_published,idx_clientid_lft	idx_client_id_parent_id_alias_language	5	\N	3237	58.97	Using index condition; Using where; Using filesort
1	SIMPLE	e	\N	eq_ref	PRIMARY	PRIMARY	4	adm_metalocator_com.m.component_id	1	100.00	Using where

[mbabker]

Is the params field still necessary in this query given the new lazy load approach?

Yes. The params field's data is just lazy loaded into a Registry instance, so the performance hit of doing that is at first read of a menu item's parameters instead of arbitrarily at the database load. If it weren't loaded as part of that base query you run into a potential N+1 situation where each individual menu item's params has to be separately queried.

[csthomas]

I wanted to add the index in order to use it. You query use another index idx_client_id_parent_id_alias_language by default.

If you can delete for a while idx_client_id_parent_id_alias_language you should see the difference.

Your query find 3237 rows unordered and then start ordering it by lft column. It takes a lot of time.
If your query will use the new index then the ordering will not be needed because rows will be scan in lft order. And this will be benefit.

The other subject is to remove params column from query and it is a good point.

[metalocator]

@csthomas The query execution speed isn't an issue at this point, it's the size of the response. Even if the query ran in 0s, we'd still be dealing with the overhead of the size of the response.

[csthomas]

If it weren't loaded as part of that base query you run into a potential N+1 situation where each individual menu item's params has to be separately queried.

Yes, this way is not good.

Maybe we should have a two columns params (basic params) and params2:
the first column will be used for routing and a few other things and params2 used only when we are on current URL.

This way we can load basic params always and params2 in separate query.
This is only an idea.

[csthomas]

@metalocator
BTW I suggest you remove the html data from the menu parameters and put them somewhere else.

[kustaag]

If I understand correctly, the problem with slowing down the operation of a site that has a large number of menus has not been resolved? In my example, this is Joomla 4.2 where 70.000 menus are used (cities, categories, etc.) and an empty main page loads in 2 seconds, it's very slow. Please tell me, in the new version of Joomla (4.2), is there any solution so that when loading the framework, all menus from the database are not loaded? maybe there is some solution?