kernel/module.php のキャッシュが効いてないバグ

Date 2007-03-06 03:26:48 | Category: XOOPS

in englishin japanese
サブメニューの実験をやっていて気づいたのですが、あるモジュールの xoops_version.php が2度も3度も読み込まれてしまいます。xoops_version.phpにロジックが書いてあるモジュールも多いので、二重読込はそのままオーバーヘッドになります。

ただ、kernel/module.php を見ると、loadInfo() メソッド自体はちゃんと$modversionをキャッシュしてますし、getInfo()もそのキャッシュを利用してます。

であれば犯人はどこだろう、と調べてみてビックリしました。モジュールハンドラのモジュールオブジェクトキャッシュが事実上機能してなかったんですね(参照で返すべきところをコピーで返していた、なぜかget()内のローカルstatic変数で持っていた等、かなりボロボロ)。モジュールオブジェクトが次々と作られてしまうため、モジュールオブジェクト毎に、$modversion をキャッシュしても意味がなかったのです。

とりあえず下のようにパッチしてみました。

これで、必要数以上のモジュールオブジェクトが生成されることはなくなり、xoops_version.php の2重読込もなくなったように見えますが、運用上の問題がでるかどうかは分かりません。しばらくこのサイトで、このコードで動かしてみます。

Cube Legacy 2.1RCのソースコードを読む限り、これとまったく同じ問題を抱えているようです。βくらいで気づけば良かったのですが、今からだとちょっと難しいでしょうか。メモリまたは速度に効いてきそうではありますが。

*** module.php.orig     Mon May  1 11:37:28 2006
--- module.php  Tue Mar  6 03:12:49 2007
***************
*** 316,328 ****
       */
      function &get($id)
      {
-         static $_cachedModule_dirname;
-         static $_cachedModule_mid;
          $ret = false;
          $id = intval($id);
          if ($id > 0) {
!             if (!empty($_cachedModule_mid[$id])) {
!                 return $_cachedModule_mid[$id];
              } else {
                  $sql = 'SELECT * FROM '.$this->db->prefix('modules').' WHERE m
id = '.$id;
                  if ($result = $this->db->query($sql)) {
--- 316,326 ----
       */
      function &get($id)
      {
          $ret = false;
          $id = intval($id);
          if ($id > 0) {
!             if (!empty($this->_cachedModule_mid[$id])) {
!                 $ret =& $this->_cachedModule_mid[$id];
              } else {
                  $sql = 'SELECT * FROM '.$this->db->prefix('modules').' WHERE m
id = '.$id;
                  if ($result = $this->db->query($sql)) {
***************
*** 331,338 ****
                          $module =& new XoopsModule();
                          $myrow = $this->db->fetchArray($result);
                          $module->assignVars($myrow);
!                         $_cachedModule_mid[$id] =& $module;
!                         $_cachedModule_dirname[$module->getVar('dirname')] =&
$module;
                          $ret =& $module;
                      }
                  }
--- 329,336 ----
                          $module =& new XoopsModule();
                          $myrow = $this->db->fetchArray($result);
                          $module->assignVars($myrow);
!                         $this->_cachedModule_mid[$id] =& $module;
!                         $this->_cachedModule_dirname[$module->getVar('dirname'
)] =& $module;
                          $ret =& $module;
                      }
                  }
***************
*** 350,360 ****
       */
      function &getByDirname($dirname)
      {
-         static $_cachedModule_mid;
-         static $_cachedModule_dirname;
          $ret = false;
!         if (!empty($_cachedModule_dirname[$dirname])) {
!             $ret = $_cachedModule_dirname[$dirname];
          } else {
              $sql = "SELECT * FROM ".$this->db->prefix('modules')." WHERE dirna
me = '".trim($dirname)."'";
              if ($result = $this->db->query($sql)) {
--- 348,356 ----
       */
      function &getByDirname($dirname)
      {
          $ret = false;
!         if (!empty($this->_cachedModule_dirname[$dirname])) {
!             $ret =& $this->_cachedModule_dirname[$dirname];
          } else {
              $sql = "SELECT * FROM ".$this->db->prefix('modules')." WHERE dirna
me = '".trim($dirname)."'";
              if ($result = $this->db->query($sql)) {
***************
*** 363,370 ****
                      $module =& new XoopsModule();
                      $myrow = $this->db->fetchArray($result);
                      $module->assignVars($myrow);
!                     $_cachedModule_dirname[$dirname] =& $module;
!                     $_cachedModule_mid[$module->getVar('mid')] =& $module;
                      $ret =& $module;
                  }
              }
--- 359,366 ----
                      $module =& new XoopsModule();
                      $myrow = $this->db->fetchArray($result);
                      $module->assignVars($myrow);
!                     $this->_cachedModule_dirname[$dirname] =& $module;
!                     $this->_cachedModule_mid[$module->getVar('mid')] =& $modul
e;
                      $ret =& $module;
                  }
              }
***************
*** 496,503 ****
              return $ret;
          }
          while ($myrow = $this->db->fetchArray($result)) {
!             $module =& new XoopsModule();
!             $module->assignVars($myrow);
              if (!$id_as_key) {
                  $ret[] =& $module;
              } else {
--- 492,506 ----
              return $ret;
          }
          while ($myrow = $this->db->fetchArray($result)) {
!             $mid = intval( $myrow['mid'] ) ;
!             if( !empty( $this->_cachedModule_mid[$mid] ) ) {
!                 $module =& $this->_cachedModule_mid[$mid] ;
!             } else {
!                 $module =& new XoopsModule();
!                 $module->assignVars($myrow);
!                 $this->_cachedModule_mid[$module->getVar('mid')] =& $module;
!                 $this->_cachedModule_dirname[$module->getVar('dirname')] =& $m
odule;
!             }
              if (!$id_as_key) {
                  $ret[] =& $module;
              } else {




You can read more news at PEAK XOOPS.
http://xoops.peak.ne.jp

The URL for this story is:
http://xoops.peak.ne.jp/md/news/index.php?page=article&storyid=406