New menu system: faster and less memory

Roadmap of the new menu system:

1. New menu system to map Drupal paths to their callbacks with appropriate access checking.
2. Drawing visual aids like navigation block, primary links, tabs, breadcrumbs.
3. Making the menu editable
4. Providing an upgrade path.

This will let us focus on the task at any time. The point of this post is to stress if you apply the patch to be submitted then your Drupal will not have anything listed in 2,3,4: navigation block, primary links, tabs, breadcrumbs, editable menu and you must start with a fresh database and be ready to loose it ongoing.

Of course, I have at least the ideas (well, some code too) to get all four so Drupal 6 won't be crippled for long.

Subscribing.

Of course, I have at least the ideas (well, some code too) to get all four

Could you post some of the code, even if not in the form of a patch? I'm curious as to how this will work.

Documentation is at http://drupal.org/node/102338 .

hook_start was not called.

I needed to make an exception to the inheritance rules so that node/add/story does not inherit from node/%/% (defined in comment), the rule is that if you have two percent signs then on build time this ancestor is not taken into consideration for inheritance purposes. I hope I will get people to review and discuss such decisions.

If you want to benchmark this then remove the menu_rebuild() command from index.php. I kept it there because it's so useful when hacking menu.

we have hook_boot and hook_init as discussed on #drupal . This means that if you used hook_init, now you need to upgrade because bootstrap.inc calls hook_boot.

menu_map now assists in arriving to a drupal_not_found .

I renamed the hook invokes but not the implementations. Fixed.

Rerolled and adjusted the comments per Dries' advice.

subscribing

Another problem in system.module, system_main_admin_page seems to return halfway through, causing q=admin to white-screen.

Perhaps this is intentional and both the admin and admin/by-module above are waiting for the core of the menu system to stabalize - I'm unsure.

I really need to provide more context in IRC.

I was talking about my little test module which was thoroughly tested. That is the part I found to be solid, obviously not the admin stuff ;)

If I would be particularly bold I would mark this RTBC because dfletcher, dvessel, bdragon, chrisxj have visited every single defined path (more than 200) and quite some that are not specifically defined. Thanks guys. I just needed to do 20 or so instead of 260+ and fix the occassional error. All this is documented http://groups.drupal.org/node/2354 . Unlike in the earlier patches, the menu_get_active_help now works, its conversion was so trivial.

Sigh. This happens when our bzr mirror breaks.

That was still yesterday's patch :(

I see them on 4si.hostignition.com/d, the public demo and developer site of this patch. Register a user, because anonymous has no rights but authenticated users have every right.

I typed 4si.hostignition.com/d/node/add/story and I see 'Submit story'. Want a screenshot?

Now if you visit 'user' as anonymous you get access denied -- as I intended. The login page is user/login.

Removed debug code (the menu_rebuild in index.php makes debugging easier).

watchdog_help was lost due to collateral damage.

Heine, how do I need to read/interprete these results? What format are they in?

I think maybe Heine meant:

and

... where 'pre' is 'pre-patch', 'post' is 'post-patch', and the first dataset is with only the default enabled core modules.

Please correct me if I'm wrong! I calculated percentages to compare—if it's really at least 20% decrease in page time / request rate, that's very snappy.

And Paul's interpretation is quite right.

I haven't tested this patch yet, but I've read the whole thing, and it looks really seriously amazing. You're a genius, chx! (but hey, we already knew that).

My biggest concern is that there doesn't seem to be proper support for non-integer wildcards in menu paths. E.g. in the patched version of node_menu(), there should just be one path for all the custom node type editing pages: admin/content/types/%. But instead, there's a foreach loop that's defining a path for each node type. This is not good: we need to be able to use strings as wildcards as well! I'm sure there will be plenty of other places, in core and contrib, where strings will need to be used as wildcards.

The binary tricks in menu_get_ancestors() are all voodoo to me, but if they work as advertised, then I say bring it on!!

There is nothing in there that would force % to be numeric. It can be whatever you want. However, in *most* cases it is a database ID and then you call map callback upon it and the default (but only the default!) map callback indeed checks for is_numeric. But noone forces you to define a map callback or even if you do, feel free to use your own.

Some suggested earlier using %d and %s but this could lead to huge database queries, with 728 maximal possibilites in the menu query instead of the 63 we currently have.

The maths is not voodoo by far (just wait until the hook registry/ordering patch hits with DFS of graph theory inside). http://en.wikipedia.org/wiki/Bitwise_operation will explain. It's rather trivial, IMO.

I think I should give credit before I get all. The patch was inspired by a mail from JonBob, I got great ideas in Brussels from Merlin, Adrian, Moshe and Dries (I hope I have not left out any of you guys), and later Steven helped straight out ancestors/parents. Only the binary maths and map callback are my ideas. dfletcher, dvessel, bdragon, chrisxj and Heine took great pains to test it througly.

As per Dries' request so that Drupal remains usable after this is commited: we now have the navigation block. I am extremely unwilling to bring more into this patch.

Heh! I forgot out access checking (thanks ChrisKennedy and Heine).

menu.inc uses int2vancode which resided in comment.module. No more.

Reroll.