Cache Functions Suggestions

[eluser]section31[/eluser]
I have a few suggestions for 2 of the cache functions in the output library.
NOTE: I'm still new to CI.

I will discuss the following function in the Output.php Library.
_display_cache() and _write_cache()

===============================================================================

_display_cache() in Output.php

Replace

Code:

if ( ! @file_exists($filepath))
{
    return FALSE;
}
if ( ! $fp = @fopen($filepath, 'rb'))
{
    return FALSE;
}
    
flock($fp, LOCK_SH);

$cache = '';
if (filesize($filepath) > 0)
{
    $cache = fread($fp, filesize($filepath));
}

flock($fp, LOCK_UN);
fclose($fp);

With

Code:

if (!@file_exists($filepath) || !($cache = @file_get_contents($filepath))) {
    return false;
}

Why
file_get_contents According to the php manual is much more efficient and is the preferred way of reading contents of a file into a string. Under my tests I noticed a 50-100KB in memory savings.

===============================================================================

_write_cache() in Output.php

Replace

Code:

$cache_path .= md5($uri);

if ( ! $fp = @fopen($cache_path, 'wb'))
{
    log_message('error', "Unable to write cache file: ".$cache_path);
    return;
}

$expire = time() + ($this->cache_expiration * 60);

flock($fp, LOCK_EX);
fwrite($fp, $expire.'TS--->'.$output);
flock($fp, LOCK_UN);
fclose($fp);
@chmod($cache_path, 0777);

With

Code:

$cache_path .= md5($uri);
$tmp_file = $cache_path . md5(uniqid(rand(), true));

if ( ! $fp = @fopen($tmp_file, 'wb'))
{
    log_message('error', "Unable to write cache file: ".$tmp_file);
    return;
}

$expire = time() + ($this->cache_expiration * 60);
fwrite($fp, $expire.'TS--->'.$output);
fclose($fp);
rename($tmp_file, $cache_path);
@chmod($cache_path, 0777);

Why
For a web site that gets hit a lot, file locks will restrict other processes from reading the cache because it's locked by a single process.
We can get around this by writing to a temporary file and once complete, we rename the file to the cache filename.
rename() acts on the file inode so it's fast and efficient

===============================================================================

What do you all think?