Recently I had a problem on a site where upon menu router rebuilds, the site would WSoD because it ran out of memory. Some research led to the conclusion that this was almost entirely to the views_menu_alter() hook implementation, which not only loads all views (I understand why that's necessary with the current architecture, since we otherwise have no way of finding out which views use a menu hook), but also initializes all of their handlers (because it needs to know the contextual filters), the latter of which is the real problem here, consuming more than a MB for some views.

At the end of the function, someone (git blame only gives me "The Great Git Migration") seems to have thought about possibly high memory consumption and added code to destroy all those views again. However, at that point the damage is already done. I've tracked memory usage throughout the function on that site (240 views with a total of 432 displays using hook_menu). The results:

function views_menu_alter(&$callbacks) {
  $our_paths = array();
  // Usage: 29.8 MB
  $views = views_get_applicable_views('uses hook menu');
  // Usage: 179.18 MB
  foreach ($views as $data) {
    list($view, $display_id) = $data;
    $result = $view->execute_hook_menu($display_id, $callbacks);
    // Usage: climbing up to 378.62 MB
    // …
  }

  // Save memory: Destroy those views.
  foreach ($views as $data) {
    list($view, $display_id) = $data;
    $view->destroy();
  }
  // Usage: 179.36 MB
}

So, loading all the views (and clones for each relevant display) takes about 150 MB, while initializing all their handlers takes 200 MB, all of which can be freed again by destroying the views.

The question: why don't we just destroy the views in the loop? I see no reason at all why we should be required to keep them, and it helps to save tons of memory for sites with many views.
If there's a real reason not to do this, I'd like to know it. Then we might need a more elaborate solution to work around this problem. More elaborate solutions would also be possible to avoid loading all the views with clones at once – i.e., more or less inlining views_get_applicable_views() (which doesn't seem to be used anywhere else anyways), potentially lowering the memory consumption by another large chunk, but I think this very simple solution would be a great start already.

Comments

drunken monkey’s picture

Status: Active » Needs review
StatusFileSize
new779 bytes

(Almost trivial) Patch attached.

valthebald’s picture

StatusFileSize
new818 bytes

Even (almost trivial) patch can be improved!
We do need to destroy view data, but it can be done within the same loop

chris matthews’s picture

The 3 year old patch in #2 to views.module applied cleanly to the latest views 7.x-3.x-dev and if still relevant needs to be reviewed.