phpBB • Free and Open Source Forum Software

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):

Code: Select all: 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.