Efficiency plumetting with large number of menu items

I think the array_unique() is pretty specific to workbench_access here, and it's apparently the heaviest operation, so although big trees are something difficult to handle, there must be a way to improve this here.

And indeed, it is separate from caching, <troll target="_self">caching is only an optimization mechanism for sissies</troll>

Hi agentrickard,

Sorry for the delay and thanks for the patch. It took me a while to get the testing environment quiet enough to isolate the effect of the patch, but it seems like I got it consistent enough now to say that it really improves the processing time for generating the tree.

The process I used (the one that seems to better isolate the tree processing time) was to run drush, then load directly the "page add" page (because it includes the huge tree as a select box).
In both cases (with and without patch) it took around 166MB in memory to generate the full page. The processor usage, however, changed considerably, with 5.2s without patch and 2.6s with patch (and this repeated 6 times on each side once other people stopped using the test infrastructure).

I'm attaching xhprof outputs both without and with patch so you can see a little bit of detail. Note (in particular) that the number of functions calls dropped from 135,826 to 128,916. This might *just* be because you stopped using array_unique() and array_merge() and might not mean much in reality, but the processor times are there to show that something is really making the load drop.

Thanks a lot for that! I'm still not sure how much data the recursive loop has to store (I would tend to recommend you keep a flat array with the data somewhere, and build the recursive tree only with the item IDs), but I stumbled upon array_merge_recursive() and thought maybe that moved the hassle of merging in C and would be more efficient? Don't know, just saying...

I'm happy to run more tests anytime (giving a backup is not a possibility right now), although I might have to mak you wait a little before I can.

I agree on the memory side. Actually the memory problem can be reduced massively by using cache (this is just to avoid re-generating the menu every time).

In particular, I think the caching mechanism could be improved. I was thinking about how you are building the menu right now, and as I was saying I thing you could keep a flat table with the info in memory, then build a complete tree (instead of section-specific trees) per role.

I reckon most Drupal websites (even with a complex workbench_access config) should not have much more than 10 roles at any time, and storing *just* the hierarchy for each role would probably be very small in memory (probably not above 10K for the hierarchy of a 4000 items-strong tree).
In parallel, having the corresponding hierarchical menu's data as a flat tree would probably be around 500 bytes per entry * 4000 =~ 2M? (I know, very bold estimations for now) That would be much less impressive than the memory actually required to process it each time.

When using the tree for any workbench_access requirement, you would use the hierachical tree for the corresponding role and just query the flat table (in memory and common to all roles) using the mlid as index.

What do you think?

Well, that could be a solution. Like put an option (off by default) somewhere in Workbench_access's config that says:

• Enable menu cache by user
• This option is particularly useful when dealing with very large trees and a reasonnable amount of editors, but requires sufficient cache memory to be reserved for this. With this option enabled, workbench access will store a specific image of every generated menu for every user (anonymous users being considered one single user), thus boosting the generation time of the menus. For maximum efficiency, it is recommended to use this option with some additional caching module like APC or Memcache.

I would really love that (although I'm really nothing of a Drupal developer, just a generic PHP dev, so can't help much in the implementation).

Then, if this option is enabled, entering workbench_access_build_tree() (reusing the suggested ugly patch base above):

$caching = false;
$cache_suffix = '';
if ($depth === -1 && variable_get('workbench_access_user_tree_cache_enabled',0)==1 && 1 === count($sections)) {
    // Change - for _ in order to avoid variable naming problems
    $cache_suffix = $uid.str_replace('-','_',$sections[0]); //don't know how to get uid, but you sure do
    // Store in menu_custom cache. Should be defined as one of the bins
    // managed by memcached.
    $cached_tree = cache_get('workbench_access_tree_built_'.$cache_suffix, 'menu_custom');
    if (isset($cached_tree->data)) {
      return $cached_tree->data;
    }
    $caching = true;
  }
  // ... the rest of the function here
  if ($caching == true) {
    cache_set('workbench_access_tree_built_'.$cache_suffix, $tree, 'menu_custom');
  }

(it would still require removing all the cached menus on menu update though)