Le core et le cache et les caches inutiles

Share and options

Précedement dans un article parlant d'un cas concret de mauvaise gestion de cache j'ai évoqué le hook_hook_info() ; En continuant ma lecture de code sur le sujet, je me suis aperçu d'une autre erreur de gestion de cache de la part des développeurs du core : les cache inutiles.

Petite note avant de commencer : mettre en place du cache quand c'est nécessaire, c'est bien, mettre en place du cache quand ce n'est pas utile, c'est la pire des choses à faire. La pire des choses à faire pour plusieurs raisons :

  • Ça complexifie le code ;
  • Dans le cas de Drupal, ça va taper un serveur distant ;
  • Ça prouve que l'auteur de ce code :
    1. N'a procédé à aucune mesure avant d'écrire son code ;
    2. L'a fait site à une intuition fausse, et en aucun cas n'en a vérifié la véracité.
  • Trop de cache tue le cache ! Et qui y joue s'y pique, et peut risque entre autres cache bloats et problèmes surprises ;
  • Et puis c'est pas beau.

1. Analyse du code

Dans l'article précédent, je parlais donc du des groupes de hooks et du fameux hook_hook_info() qui permet de les définir. En suivant le code, j'ai lu ça  (module.inc, ligne 717):

/**
* Retrieve a list of what hooks are explicitly declared.
*/
function module_hook_info() {
  // This function is indirectly invoked from bootstrap_invoke_all(), in which
  // case common.inc, subsystems, and modules are not loaded yet, so it does not
  // make sense to support hook groups resp. lazy-loaded include files prior to
  // full bootstrap.
  if (drupal_bootstrap(NULL, FALSE) != DRUPAL_BOOTSTRAP_FULL) {
    return array();
  }
  $hook_info = &drupal_static(__FUNCTION__);

  if (!isset($hook_info)) {
    $hook_info = array();
    $cache = cache_get('hook_info', 'cache_bootstrap');
    if ($cache === FALSE) {
      // Rebuild the cache and save it.
      // We can't use module_invoke_all() here or it would cause an infinite
      // loop.
      foreach (module_list() as $module) {
        $function = $module . '_hook_info';
        if (function_exists($function)) {
          $result = $function();
          if (isset($result) && is_array($result)) {
            $hook_info = array_merge_recursive($hook_info, $result);
          }
        }
      }
      // We can't use drupal_alter() for the same reason as above.
      foreach (module_list() as $module) {
        $function = $module . '_hook_info_alter';
        if (function_exists($function)) {
          $function($hook_info);
        }
      }
      cache_set('hook_info', $hook_info, 'cache_bootstrap');
    }
    else {
      $hook_info = $cache->data;
    }
  }

  return $hook_info;
}

On voit très aisément ici que l'auteur de ce code à décider de placer un cache du résultat de l'exécution du hook_hook_info() sur tous les modules. Ce que je peux comprendre ici, dans un sens, il n'a probablement pas envie de lancer 100 appels aux fonctions function_exists() et array_merge_recursive() successivement.

Dans le doute, ce hook pourrait être appellé un grand nombre de fois, peut être. Cependant, en pratique, je vais vous laisser réfléchir sur ce bout de code (fonction module_implements(), fichier module.inc ligne 658):

  // Fetch implementations from cache.
  if (empty($implementations)) {
    $implementations = cache_get('module_implements', 'cache_bootstrap');
    if ($implementations === FALSE) {
      $implementations = array();
    }
    else {
      $implementations = $implementations->data;
    }
  }

  if (!isset($implementations[$hook])) {
    // The hook is not cached, so ensure that whether or not it has
    // implementations, that the cache is updated at the end of the request.
    $implementations['#write_cache'] = TRUE;
    $hook_info = module_hook_info();
    $implementations[$hook] = array();
    $list = module_list(FALSE, FALSE, $sort);
    foreach ($list as $module) {
      $include_file = isset($hook_info[$hook]['group']) && module_load_include('inc', $module, $module . '.' . $hook_info[$hook]['group']);
      // Since module_hook() may needlessly try to load the include file again,
      // function_exists() is used directly here.
      if (function_exists($module . '_' . $hook)) {
        $implementations[$hook][$module] = $include_file ? $hook_info[$hook]['group'] : FALSE;
      }
    }
    // Allow modules to change the weight of specific implementations but avoid
    // an infinite loop.
    if ($hook != 'module_implements_alter') {
      drupal_alter('module_implements', $implementations[$hook], $hook);
    }
  }

On peut voir ici que la fonction module_hook_info() n'est appellée que lorsque le cache n'a pas déja été construit. Le cache de la fonction hook_hook_info() n'est donc utile que lorsque que le cache de module_implements() n'est pas complet, et donc lors qu'une reconstruction d'un cache plus basse dans la pile d'exécution : on a donc un cache qui précède un autre, et le rend superflu dans ce chemin d'exécution précis.

On aurait pu imaginer que cette fonction puisse être appellée à d'autres endroits, donc je suis aller chercher si c'était le cas :

[pounard@guinevere] /var/www/blog-d7/www
>  grep -Rn "module_hook_info(" *
includes/module.inc:603:  $hook_info = module_hook_info();
includes/module.inc:673:    $hook_info = module_hook_info();
includes/module.inc:717:function module_hook_info() {
modules/system/system.api.php:51: *   Information gathered by module_hook_info() from other modules'

En effet il s'avère qu'un autre appel à cette fonction est fait, et pas des moindres, puisqu'il s'agit de la fonction module_hook() qui est potentiellement appellée un grand nombre de fois sur votre page. Mais là encore je ne comprends pas une chose : pourquoi ne se servent-ils pas du cache de la fonction module_implements() dans la fonction module_hook() ?! Je trouve ça abbérant que le coeur conserve de multiples caches pour stocker la même information.

2. De simples suppositions ne suffisent pas

Une autre considération est à prendre en compte ici : même si vous avez 200 modules installés sur votre site, 200 appels à la fonction function_exists() seront toujours plus rapide qu'une requête SQL ou tout autre requête vers un serveur distant.

Donc pour aller plus loin, et parce que cette fonction sera appellée à l'exécution de toutes les pages, j'ai décidé de passer au profiling bas niveau. Pour ce faire, j'ai utilisé XDebug (certains préfèreront XHProf, choix tout à fait respectable). Je vais me passer de vous montrer quelconque code ou traces ici, mais sur le site sur lequel j'ai testé, la requête SQL vers le cache coutaît systématiquement plus cher en temps que de reconstruire le cache de cette fonction à la volée. J'ai pris plusieurs mesures pour le vérifier.

Ce cache est désespérément inutile, et engendrera une requête vers un serveur distant qui n'a aucune raison d'être, ceci au moins une fois lors de chaque cache miss de la fonction module_implements() ou une autre fois lors de l'appel à module_hook(), donc sur chaque page.

Si quelqu'un a fait des mesures qui démontrent le contraire, merci de les partager, je serais plus que curieux de voir dans quelles conditions appeller 200 fois function_exists() serait plus lent que la trace d'exécution complète générée par un appel à cache_get(), qui quand on utilise le backend de cache par défault utilise la base de donnée et DBTng, qui faut l'avouer, a un impact significatif quant au temps CPU qu'il consomme.

Je tiens à dire que le profilage que j'ai effectué, à lui seul, n'est pas assez précis pour être prit pour argent comptant : que ce soit d'autres processus sur ma machine perturbant l'exécution ou le fait que j'ai finalement peu de modules sur mon site de test, seul un vrai test de montée en charge pourra infirmer ou confirmer ma thèse. Je doute cependant que l'auteur de ce code ne l'ait réellement fait.

3. Quelles améliorations

Tout le code de gestion des modules de Drupal commence à être aujourd'hui vieillissant, la fonction module_implements() le démontre assez bien de par son ampleur. De plus, d'autres fonctions comme module_hook() peuvent interagir avec (comme le prouvent certains commentaires dans ces fonctions). Il n'y qu'une seule réelle solution dans ce cas : réécrire toute la gestion des hooks afin que l'ensemble des fonctions utilise un cache unique plus intelligement.