The menu - does it have to be different on each page?
Hi there, In my endeavour to have better caching in Drupal. One of the things we already cache is the menu tree for each user. What we cannot cache is the rendered version of this tree (the navigation block) because it changes on each and every page. I was wondering if we need this. Couldn't we always render the complete menu and only hide the pieces we don't want to show through some CSS/JS? Sure, this would put users of non-CSS browsers maybe at an disadvantage, but is it that bad? If you are not an admin the number of additional items is rather small. One problem that has been pointed out is that we add the "active" class to the current menu item. Would there be a way around this? Caching the whole rendered menu block would have some serious performance implications. Cheers, Gerhard
Couldn't we always render the complete menu and only hide the pieces we don't want to show through some CSS/JS?
It would be a good performance boost not to have to rerender the menu for every page. There might be usability benefits too. There have been various attempts to get a fully rendered menu tree for usability purposes, so that we can have expandable tree menus, avoiding the need for frequent page reloads, see e.g. the nice_menus module and my patch to this closed issue http://drupal.org/node/29551, which did some of what might be needed. Making a single menu work on all pages should be simple enough with css and js. But with just css? We'd need to figure out some trick to get our 'expanded' and 'collapsed' classes to recognized the current location. No brilliant idea comes immediately to mind. Anyone else?
On Tue, August 29, 2006 6:12 pm, Nedjo Rogers said:
Couldn't we always render the complete menu and only hide the pieces we don't want to show through some CSS/JS?
It would be a good performance boost not to have to rerender the menu for every page.
There might be usability benefits too. There have been various attempts to get a fully rendered menu tree for usability purposes, so that we can have expandable tree menus, avoiding the need for frequent page reloads, see e.g. the nice_menus module and my patch to this closed issue http://drupal.org/node/29551, which did some of what might be needed.
Making a single menu work on all pages should be simple enough with css and js. But with just css? We'd need to figure out some trick to get our 'expanded' and 'collapsed' classes to recognized the current location. No brilliant idea comes immediately to mind. Anyone else?
The only way I know of requires knowing the full list of possible expandable entries in advance. That means either auto-generated CSS in the page itself, or an extremely large CSS setup, or both. However, that said: <body id="path_a"> <ul> <li class="leaf">Foo</li> <li class="expandable path_a">A <ul> <li class="leaf">Foo</li> </ul> </li> <li class="expandable path_b">B <ul> <li class="leaf">Foo</li> </ul> </li> <ul> </body> .expandable { display: none; } #path_a .path_a, #path_b .path_b { display: block; } Building up the list of ids and classes for that last rule, though, would be difficult if not impossible. Maybe just impractical. OK, I'll toss the above (untested and off the cuff) snippet out in case anyone else has a different brilliant idea. :-) --Larry Garfield
Nedjo Rogers wrote:
Couldn't we always render the complete menu and only hide the pieces we don't want to show through some CSS/JS?
It would be a good performance boost not to have to rerender the menu for every page.
There might be usability benefits too. There have been various attempts to get a fully rendered menu tree for usability purposes, so that we can have expandable tree menus, avoiding the need for frequent page reloads, see e.g. the nice_menus module and my patch to this closed issue http://drupal.org/node/29551, which did some of what might be needed.
Making a single menu work on all pages should be simple enough with css and js. But with just css? We'd need to figure out some trick to get our 'expanded' and 'collapsed' classes to recognized the current location. No brilliant idea comes immediately to mind. Anyone else? JQuery? ;)
Caching the whole rendered menu block would have some serious performance implications.
unless i am missing something, this focus on rendering is misplaced. you still have calculate the whole menu tree to find the right path for callback and breadcrumb. thats where the performance cost is. building the rendering isn't too bad. furthermore, most big sites just disable navigation block and do own navigation using a different menu or separate from menu system. now if we could determine callback without building the menu tree we would be in real fine shape. -moshe
Had major menu performance issues as well, following changes made an incredible difference with 5,000 menu items and 50,000 nodes 1) _menu_sort gets called recursively so "$menu = menu_get_menu();" was a huge problem, switched to the global $_menu instead. 2) menu_get_menu() was altered to cache (APC) menus by ROLES & locale not user. When retrieved from cache the user specific callbacks and stuff gets fixed up As far as I can tell when using APC cache with PHP5 there is no serialize/unserialize penalty when using simple types (not objects). Since menu is a simple array this helps. This is nothing more than a hack but I had little choice at the time. The APC cache needs to be manually cleared in order for menu changes to show but that seems to be the only downside so far. function _menu_sort($a, $b) { // $menu = menu_get_menu(); global $_menu; . . . function menu_get_menu() { global $_menu; global $user; global $locale; if (!isset($_menu['items'])) { // _menu_build() may indirectly call this function, so prevent infinite loops. $_menu['items'] = array(); $menukey = implode(':',array_keys($user->roles)); if ($user->uid == 1) { $cid = "adminmenu"; } else { $cid = "menu:$menukey:$locale"; } if ($_menu = apc_fetch($cid)) { //loaded menu from cache need to fixup user specific menu $newpath = 'user/'.$user->uid; $matched = preg_grep('/user\/\d/',array_keys($_menu['path index'])); $result = count($matched); if ($result == 1) { $target = array_pop($matched); $targetvalue = $_menu['path index'][$target]; unset($_menu['path index'][$target]); $_menu['path index'][$newpath] = $targetvalue; $_menu['callbacks'][$newpath] = $_menu['callbacks'][$target]; unset($_menu['callbacks'][$target]); $_menu['visible'][$targetvalue]['path'] = $newpath; $_menu['items'][$targetvalue]['path'] = $newpath; } } else { _menu_build(); apc_store($cid,$_menu,100000); } // Make sure items that cannot be cached are added. _menu_append_contextual_items(); // Reset the cached $menu in menu_get_item(). menu_get_item(NULL, NULL, TRUE); } return $_menu; } George Kappel
-----Original Message----- From: development-bounces@drupal.org [mailto:development-bounces@drupal.org] On Behalf Of Moshe Weitzman Sent: Tuesday, August 29, 2006 6:28 PM To: development@drupal.org Subject: Re: [development] The menu - does it have to be different on eachpage?
Caching the whole rendered menu block would have some serious performance implications.
unless i am missing something, this focus on rendering is misplaced. you still have calculate the whole menu tree to find the right path for callback and breadcrumb. thats where the performance cost is. building the rendering isn't too bad. furthermore, most big sites just disable navigation block and do own navigation using a different menu or separate from menu system.
now if we could determine callback without building the menu tree we would be in real fine shape.
-moshe
Op woensdag 30 augustus 2006 01:28, schreef Moshe Weitzman:
now if we could determine callback without building the menu tree we would be in real fine shape.
Yea. Though interesting idea -rendering the whole tree- I feel it is "looking in the wrong area for a solution". The problem is not suc much the building of the whole menu vs building a part, the problem is that the menu-system is not designed to be optimised very well. Rendering a full menu brings in lots of problems, so I give it a big -1. * I looked around on some of my installs and found that some have over 600 menu items. I'd hate to see them go over the line for every pageview. * Many modules offer "dynamic" items. Including these in a static menu can let it grow over 70.000 items (that is the max I counted in a menu I have, with views and freetagging). No way that that will go over the line. A rough calculation brings me a menu HTML chunk of over half a Meg. * Depending on CSS and JS, greatly reduces themability and flexibility. Changing a few bits of CSS will require intimite knowledge of the way Drupal does the hiding/showing. And lots of nice, simple CSS tricks, such as hovers, background-images will no longer be possible, or else require a lot of CSS hacks, in order to co-exist with a core CSS system to hide menu parts. Advanced theme overrides (such as sliding door tricks, or putting the menu in tables etc) will become very cumbersome. I think we should look at a more robust solution, such as dropping the /path/hierarchy and embracing a parent-child relation system that lives in the database only. But I am sure people have better ideas than this :) Bèr
On Wed, 30 Aug 2006, B�r Kessels wrote:
Op woensdag 30 augustus 2006 01:28, schreef Moshe Weitzman:
now if we could determine callback without building the menu tree we would be in real fine shape.
Yea. Though interesting idea -rendering the whole tree- I feel it is "looking in the wrong area for a solution". The problem is not suc much the building of the whole menu vs building a part, the problem is that the menu-system is not designed to be optimised very well.
(Thought provoking questions, hopefully) WWFAPID? (What would FormsAPI do?) Would doing a more formsapi-ish approach make the code easier to read? Slower? Faster? Why isn't the menu *always* a semi-static thing that modules get a chance to alter? Why is the menu responsible for paths, really? Wouldn't a seperate system for paths that could tell the menu to "expand to root" certain items work too? Couldn't modules take the menu object and tell it to expand the module's relevant roots, and then the module mark its own items as being active? Why does the menu try to do everything itself? Why not delegate some stuff to the modules? Why do modules publish individual menu items? Why not publish local roots and a callback for the menu system to get the children when needed? Wouldn't this make (taxonomy|category|whatever)_menu modules less bug prone? Is there any reason the menu manager can't just become a *service* to modules? (i.e. an interface to let the admin (user even?) organize the children under module local roots?) Is the ability to move the originals around such a good idea? Why not make it easier to duplicate items instead? Could "alternate paths to this item" be stored in the menu item?
Rendering a full menu brings in lots of problems, so I give it a big -1.
* I looked around on some of my installs and found that some have over 600 menu items. I'd hate to see them go over the line for every pageview. * Many modules offer "dynamic" items. Including these in a static menu can let it grow over 70.000 items (that is the max I counted in a menu I have, with views and freetagging). No way that that will go over the line. A rough calculation brings me a menu HTML chunk of over half a Meg. * Depending on CSS and JS, greatly reduces themability and flexibility. Changing a few bits of CSS will require intimite knowledge of the way Drupal does the hiding/showing. And lots of nice, simple CSS tricks, such as hovers, background-images will no longer be possible, or else require a lot of CSS hacks, in order to co-exist with a core CSS system to hide menu parts. Advanced theme overrides (such as sliding door tricks, or putting the menu in tables etc) will become very cumbersome.
I think we should look at a more robust solution, such as dropping the /path/hierarchy and embracing a parent-child relation system that lives in the database only. But I am sure people have better ideas than this :)
B�r
--
From the mailer of |_|0|_| B R A N D O N B E R G R E N |_|_|0| T e c h n i c a l |0|0|0| G e n e r a l i s t ( C S )
Good points, see comments below Brandon Bergren write:
... (Thought provoking questions, hopefully)
WWFAPID? (What would FormsAPI do?) $menu['menu'] = array( '#type' => 'menu', '#path' => array ('menu', 'yet', 'another', 'path', 'etc'), '#title' => t('Menu'), '#tree' => TRUE, '#collapsible' => TRUE, '#collapsed' => TRUE, ); $menu['menu']['item'] = array(' '#type' => 'node', '#nid' => 1337, '#title' => t('Node'), '#description' => t('A node item'), '#weight' => 5, ); $menu['menu']['item2'] = array(' '#type' => 'page', '#callback' => 1337, '#title' => t('A page'), '#path' => array(t('translatable'), t('path')), );
Would doing a more formsapi-ish approach make the code easier to read? Slower? Faster? Not sure
Why isn't the menu *always* a semi-static thing that modules get a chance to alter? If the code above is loaded in bootstrap it will be
Why is the menu responsible for paths, really? Wouldn't a seperate system for paths that could tell the menu to "expand to root" certain items work too? Couldn't modules take the menu object and tell it to expand the module's relevant roots, and then the module mark its own items as being active?
Why does the menu try to do everything itself? Why not delegate some stuff to the modules? IMO paths is rather related to menu items, or it should be... Why do modules publish individual menu items? Why not publish local roots and a callback for the menu system to get the children when needed? Wouldn't this make (taxonomy|category|whatever)_menu modules less bug prone?
Is there any reason the menu manager can't just become a *service* to modules? (i.e. an interface to let the admin (user even?) organize the children under module local roots?) Is the ability to move the originals around such a good idea? Why not make it easier to duplicate items instead? Could "alternate paths to this item" be stored in the menu item? Duplicates is not a good idea IMO, but it should be easier to duplicate/create shortcuts to menu items, without messing up the entire system
On 30 Aug 2006, at 8:25 PM, Johan Forngren wrote:
WWFAPID? (What would FormsAPI do?) $menu['menu'] = array( '#type' => 'menu', '#path' => array ('menu', 'yet', 'another', 'path', 'etc'), '#title' => t('Menu'), '#tree' => TRUE, '#collapsible' => TRUE, '#collapsed' => TRUE, ); $menu['menu']['item'] = array(' '#type' => 'node', '#nid' => 1337, '#title' => t('Node'), '#description' => t('A node item'), '#weight' => 5, ); $menu['menu']['item2'] = array(' '#type' => 'page', '#callback' => 1337, '#title' => t('A page'), '#path' => array(t('translatable'), t('path')), );
a couple of things for this. You would still sit with the may_cache issue, in that every run would need 2 calls, because it can't be cached. What i plan to do is make fapi elements possibly be callbacks, which means any property can be a function that can instead be executed, along with variables that can be used in the call. What this would mean is that the instruction to check the function can be cached, but not the final value.. an example of this would be the access check, which currently needs to be cached per role because of permissions. . In the following case : $menu['node']['%d'][edit'] = array(' '#type' => 'page', '#callback' => 'drupal_get_form', '#callback_arguments' => callback('arg', 1), // returns an array, so you can do callback(something) + callback(something) '#access' => callback('user_access', 'some_permission'), '#title' => t('A page'), '#path' => array(t('translatable'), t('path')), ) Take for example if you had the entire menu loaded into memory (and unserialising it is quite expensive too). When loading up the system, is can go : $section &= $menu; $parts = expode('/', $_GET['q']); foreach ($parts as $part) { $x++; // this if statement will almost surely be incorrect, but it has the general idea if (isset($section[$part]) || (is_numeric($part) && isset($section ['%d'])) || (isset($section['%s']) ) { // finds all callback arrays, and trigger them trigger_callbacks($section[$part]); if (!$section['#access']) { drupal_access_denied(); break; } else { //success, we have access to this item $section &= $section[$part]; $found_part[] = $part; } } What you will have here, is : a) the stuff stored in the database not being role specific, and one data structure is capable of serving the entire system b) expensive 'path index' building not needed. c) no longer needing may_cache, as the callbacks can be stored in the database, but the final values can't. d) Only the required parts of the tree are touched. You could possibly select only the part of the tree you are interested in .. in this example : node, node/1, node/%d, node/%s, node/1/edit, node/1/%d, node/1/%s, node/%d/edit etc. So you could put those all in an array, and search for just those menu items, and also sort them that way for traversal. Doing such a search on a textfield could be expensive, but earl mentioned that we could possibly just do md5's of the data in the database, and just doing a search on md5sums in (list of integers). This is only part of the battle though, as matching paths to callbacks is a much lighter problem than actually displaying the menu. But I think , that if you had the md5 of each of the parent elements of the menu items (and you have the entire path to your current location), you could create a single sql query to retrieve only the necessary levels from the tree.. and each of these layers could very possibly be cached. (ie: get all the elements directly in the root, all the elements in node/ , all the elements in node/1 and all the elements underneath node/1).
Would doing a more formsapi-ish approach make the code easier to read? Slower? Faster?
Not sure
Eh. i'm not sure either, but I'm confident with the tricks I outlined, we should be able to at least get the bootstrap requirements lowered, since every load of drupal loads the _ENTIRE_ menu structure with _ALL_ the information needed to render that mammoth block. Not to mention, that all the menu items are always loaded into drupal (it was the same case with the paths, and made them perform quite badly)
At 12:09 AM +0200 30/8/06, Gerhard Killesreiter wrote:
In my endeavour to have better caching in Drupal. One of the things we already cache is the menu tree for each user. What we cannot cache is the rendered version of this tree (the navigation block) because it changes on each and every page.
I was wondering if we need this. Couldn't we always render the complete menu and only hide the pieces we don't want to show through some CSS/JS?
I've built sites with upwards of 1000 menu items and loading an entire menu tree on each page view would be slow and costly in terms of bandwidth. Of course building a menu tree of that scale isn't something you want to do often, and I wouldn't consider it viable for a site with anything but anonymous and admin users. I've done some hacking and there is a big performance boost to menu building to be gained by sanitise-on-input rather than the current system that sanitises all the data in the menu tree every time it's built. I got half-way through hacking into the menu system a caching mechanism so that it stored the user's original input and also cached a sanitised version, but that turned out to not really be workable. I really think there are huge performance benefits to switching all of Drupal to a sanitise-on-input model, but I can see the other side of the argument... the usability of Drupal is much-enhanced by storing verbatim user input. Like when you write a PHP snippet as content but forget to turn on "PHP code". If that was sanitised (i.e. stripped) I'd have to write that code again. With the current model I just have to turn PHP code on, which is nice. ...R.
participants (10)
-
adrian rossouw -
Brandon Bergren -
Bèr Kessels -
George Kappel -
Gerhard Killesreiter -
Johan Forngren -
Larry Garfield -
Moshe Weitzman -
Nedjo Rogers -
Richard Archer