Uploaded image for project: 'phpBB'
  1. phpBB
  2. PHPBB-2722

Code waste in acm_file.php

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • None
    • 3.0.x
    • Caching (ACM)
    • None
    • PHP Environment:
      Database:

      The phpBB built-in file caching layer is inefficient. It uses a combination of serialize(), implode() and str_replace() to export an array, when PHP contains a built-in function that will perform the same job with better speed: var_export(). I've patched phpBB to use var_export(); here's the patch (not thoroughly tested, but it seems to work well):

      Index: includes/acm/acm_file.php
      ===================================================================
      RCS file: /cvsroot/phpbb/phpBB2/includes/acm/acm_file.php,v
      retrieving revision 1.42
      diff -u -r1.42 acm_file.php
      --- includes/acm/acm_file.php	7 Oct 2006 17:40:06 -0000	1.42
      +++ includes/acm/acm_file.php	3 Jan 2007 22:54:30 -0000
      @@ -71,7 +71,7 @@
       		}
       
       		global $phpEx;
      -		$file = "<?php\n\$this->vars = " . $this->format_array($this->vars) . ";\n\n\$this->var_expires = " . $this->format_array($this->var_expires) . "\n?>";
      +		$file = "<?php\n\$this->vars = " . var_export($this->vars, true) . ";\n\n\$this->var_expires = " . var_export($this->var_expires, true) . "\n?>";
       
       		if ($fp = @fopen($this->cache_dir . 'data_global.' . $phpEx, 'wb'))
       		{
      @@ -172,7 +172,7 @@
       			if ($fp = @fopen($this->cache_dir . 'data' . $var_name . ".$phpEx", 'wb'))
       			{
       				@flock($fp, LOCK_EX);
      -				fwrite($fp, "<?php\n\$expired = (time() > " . (time() + $ttl) . ") ? true : false;\nif (\$expired) { return; }\n\n\$data = unserialize('" . str_replace("'", "\\'", str_replace('\\', '\\\\', serialize($var))) . "');\n?>");
      +				fwrite($fp, "<?php\n\$expired = (time() > " . (time() + $ttl) . ") ? true : false;\nif (\$expired) { return; }\n\n\$data = " . var_export($var, true) . ";\n?>");
       				@flock($fp, LOCK_UN);
       				fclose($fp);
       			}
      @@ -291,37 +291,6 @@
       	}
       
       	/**
      -	* Format an array to be stored on filesystem
      -	*/
      -	function format_array($array, $tab = '')
      -	{
      -		$tab .= "\t";
      -
      -		$lines = array();
      -		foreach ($array as $k => $v)
      -		{
      -			if (is_array($v))
      -			{
      -				$lines[] = "\n{$tab}'$k' => " . $this->format_array($v, $tab);
      -			}
      -			else if (is_int($v))
      -			{
      -				$lines[] = "\n{$tab}'$k' => $v";
      -			}
      -			else if (is_bool($v))
      -			{
      -				$lines[] = "\n{$tab}'$k' => " . (($v) ? 'true' : 'false');
      -			}
      -			else
      -			{
      -				$lines[] = "\n{$tab}'$k' => '" . str_replace("'", "\\'", str_replace('\\', '\\\\', $v)) . "'";
      -			}
      -		}
      -
      -		return 'array(' . implode(',', $lines) . ')';
      -	}
      -
      -	/**
       	* Load cached sql query
       	*/
       	function sql_load($query)
      @@ -350,7 +319,6 @@
       		}
       
       		$this->sql_row_pointer[$query_id] = 0;
      -
       		return $query_id;
       	}
       
      @@ -376,12 +344,10 @@
       			while ($row = $db->sql_fetchrow($query_result))
       			{
       				$this->sql_rowset[$query_id][] = $row;
      -
      -				$lines[] = "unserialize('" . str_replace("'", "\\'", str_replace('\\', '\\\\', serialize($row))) . "')";
       			}
       			$db->sql_freeresult($query_result);
       
      -			fwrite($fp, "<?php\n\n/*\n" . str_replace('*/', '*\/', $query) . "\n*/\n\n\$expired = (time() > " . (time() + $ttl) . ") ? true : false;\nif (\$expired) { return; }\n\n\$this->sql_rowset[\$query_id] = array(" . implode(',', $lines) . ') ?>');
      +			fwrite($fp, "<?php\n\n/*\n" . str_replace('*/', '*\/', $query) . "\n*/\n\n\$expired = (time() > " . (time() + $ttl) . ") ? true : false;\nif (\$expired) { return; }\n\n\$this->sql_rowset[\$query_id] = " . var_export($this->sql_rowset[$query_id], true) . "\n?>");
       			@flock($fp, LOCK_UN);
       			fclose($fp);

      I did a quick check and it seems that this modified code does properly load things, and the query count certainly goes down when the cache files exist, so it seems to work.

      This should be somewhat faster.

            Acyd Burn Meik Sievertsen [X] (Inactive)
            cap'n refsmmat cap'n refsmmat
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

              Created:
              Updated:
              Resolved: