The function bootstrap_invoke_all of bootstrap.inc is called when the cache variable is set to CACHE_NORMAL. It is passed which hook to invoke on the bootstrapped modules with the following sequence.

        require_once './includes/module.inc';
        bootstrap_invoke_all('init');
        drupal_page_cache_header($cache);
        bootstrap_invoke_all('exit');

The bootstrap_invoke_all calls the module_list with the refresh set to TRUE which causes a DB select for both of these calls. Is there a reason that the DB is read twice for this? I've attached a simple patch to cause the function to refresh on the invoke of init and to not refresh thereafter.

CommentFileSizeAuthor
#7 bootstrap.inc_.patch_0.txt625 bytesAnonymous (not verified)
bootstrap.inc_.patch.txt686 bytesAnonymous (not verified)

Comments

Chris Johnson’s picture

Chris Johnson commented

Seems like a straight-forward and rational optimization to me.

dries’s picture

dries commented

We need to fix this, but I want to figure out the cleanest approach. The proposed patch is simple but awkward. It points out a flaw in the API of module_list(). We need weird code to work around the behavior of module_list(). The proper solution is to fix module_list, IMO.

Why shoudn't we be able to call module_list(FALSE, TRUE) instead of module_list(TRUE, TRUE)? When $refresh = FALSE, it should use lazy-initialization to load the list of modules when these haven't been loaded yet. The second time, it should notice that the list is already present, and return the list as is.

Can you look into this some more Earnie?

Anonymous’s picture

Anonymous (not verified) commented

This changes somewhere between 4.7.3 and HEAD

It used to be:

function bootstrap_invoke_all($hook) {
  foreach (module_list(FALSE, TRUE) as $module) {
    drupal_load('module', $module);
    module_invoke($module, $hook);
  }
}

I'll look further into module_list.

Anonymous’s picture

Anonymous (not verified) commented

Module_list is used for three different functions:

  1. A list of all modules from system whose status is 1.
  2. A list of all modules from system whose status is 1 and bootstrap is 1.
  3. A list of modules given as an argument to the function.

The refresh is used to reset the static $list with appropriate values. I'm of the opinion that we need three different functions; module_list, module_bootstrap_list and module_fixed_list. The static $list for module_list and module_bootstrap_list would only need to be refreshed if status changes for a module. There would be no static $list for the module_fixed_list.

Anonymous’s picture

Anonymous (not verified) commented

As I hack at module_list I'm finding other interesting issues. It'll be a little while before I get back to you on this. I'm approaching the module_list patch such that no API changes are made but individual helper functions are created. Later...

Anonymous’s picture

Anonymous (not verified) commented

I've created http://drupal.org/node/116820 to track the module_list change. Once that patch is accepted I will review the use of module_list to determine if the refresh argument is being used optimally.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new bootstrap.inc_.patch_0.txt625 bytes

Assuming the patch http://drupal.org/node/116820#comment-195585 is committed then all we need to do for bootstrap_invoke_all is change the refresh parameter from TRUE to FALSE.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Closed (duplicate)

The patch is contained in the all encompassing patch at http://drupal.org/node/116820#comment-205539.