I think (i could be wrong), that if you use %node in your menu hook, it will load the node as part of the menu call.

This is not ideal for a hook thats meant to be as light weight as possible to insure performance. In the function, it only makes use of the NID except a single sanity check (!$node).

Would you accept a patch to change to just accepting the NID, then check with a small SQL query (if at all)?

Comments

nunoveloso’s picture

nunoveloso commented

I am totally with a_c_m here. Will be happy to help too.

wiifm’s picture

wiifm
Primary language English
Location Wellington, NZ
commented

Yes happy to accept patches here, as long as there is validation inside the callback to ensure it is numeric, and an actual NID etc etc, this should be fine.

Thanks for your help

Would be nice to port this forward to the 7.x branch as well

dalin’s picture

dalin
he/him
Primary language English
Location 🇨🇦, 🇲🇽, 🌍
commented

I think using %node is actually necessary because we can't assume that nodes are stored in a database (could be MongoDB, etc.) node_load() will actually be faster anyway assuming that you are using entitycache + memcache (or some other non-DB caching system) (and I assume that you are using these things if this degree of performance is important to you).

wiifm’s picture

wiifm
Primary language English
Location Wellington, NZ
commented

There is truth to what Darlin is saying, once you have loaded the node once through node_load() it can be cached (drupal 7 will cache it statically for the page load at a base level). There are also alternative caching mechanisms available to help, e.g. memcache, apc etc

I don't think this was a_c_m's and nunoveloso problem though. They feel (and I semi agree), that a full node_load() for what is supposed to be a quick table update, is overkill. To be honest I never considered nodes not being in the drupal database, so if this is something that is happening on your drupal site (e.g. with a mongo db store), can you let me know in the comments ?

There are alternative lightweight bootstrap handlers out there as well, e.g. https://drupal.org/project/js - this project aims to not do a full drupal bootstrap, you can define the level you wish to go to (e.g. bootstrap to the database level, or session level etc). This might be good to integrate (no drupal 7 version though).