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?

[eluser]Mark van der Walle[/eluser]
The locks are there for a reason If you have a high traffic then you might get multiple read/write attempts on a file. The locking might slow you down a bit but will prevent corrupted cache files and php errors.

[eluser]section31[/eluser]
Lock isn't necessary using my methods.

[eluser]isaiahdw[/eluser]
Looks good section31, I'm going to play around with it and see how it compares to the unmodified CI cache functions.

[eluser]Rick Jolly[/eluser]
[quote author="section31" date="1201926914"]
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.
[/quote]
Other processes will be momentarily delayed from reading the cache while it's being written, but we're talking milliseconds aren't we?

[quote author="section31" date="1201926914"]
We can get around this by writing to a temporary file and once complete, we rename the file to the cache filename.
[/quote]
What happens when the file is overwritten by rename when another process is in the middle of reading it? I'm thinking that the reader will get corrupted data, but I really don't know.

[eluser]Elliot Haughin[/eluser]
Due to the nature of caching... one assumes that you will have a high traffic site to warrant using it.

Therefore, if you have a lot of traffic, and you're caching a page for an hour...

Over the course of one hour, you may read the file possibly 200 times, and write just once...
The time taken to write is minimal, around 10ms? - So, out of one hour, you have 10ms 'downtime' for your cache reads.

Which is 0.00027% downtime... Extremely minimal, and not really worth worrying about.
Locks will only cause this fractional locking time, and, if it only effects 2 people trying to write at the same time.

The correct way to work around it is explained here:

* 1 page is cached for an hour.

* When that cache expires, 2 people visit the site with 10ms of each other (the only time this will cause an 'issue', because otherwise, the first will have written the cache before the second requests the page).

* The first user 'reads' the cache file, sees it's expired so begins rendering the page normally with PHP/MySQL
* The second user has the same thing going on... the cache hasn't been updated by the second user.

* The first user finishes building the page, and begins writing the file, locking it.
* The second user also finishes building the page, and attempts to write it.

* Because the first user has locked the file, the second user can't write it...
* This is a problem for the second user, so you simply build a little fix to make it all work like magic.

If the second user can't write the the cache (because it is locked), simply echo the contents of the rendered page, not saving it to the cache.
The first user is dealing with the cache, so just render the output.

With CI, this is how it works...

Line 299 of Output.php:

Code:

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

The cache file is only written if it's 'really_writable' (not locked)

Line 59 of Output.php:

Code:

elseif (($fp = @fopen($file, 'ab')) === FALSE)
    {
        return FALSE;

So... if we cant write it, just render the second users' request.

Simple, and it works, beautifully.

The guys at Ellislabs have a very very good caching system built for us here, and it doesn't need any work done to it.

[eluser]isaiahdw[/eluser]
Thanks for the explanation Elliot Haughin, locking the file the way CI does it makes perfect sense now!

Would it still be better to use file_get_contents() (not fread) when reading the cache files?

[eluser]isaiahdw[/eluser]
Ah, never mind my last post about file_get_contents(). You can't (as far as I know) lock the file using file_get_contents(), so fread is required for reading the cache also.

[eluser]Derek Allard[/eluser]
Elliot. You rock. Thanks sir.

[eluser]Rick Jolly[/eluser]
[quote author="Elliot Haughin" date="1202853370"]
...
* The first user finishes building the page, and begins writing the file, locking it.
* The second user also finishes building the page, and attempts to write it.

* Because the first user has locked the file, the second user can't write it...
* This is a problem for the second user, so you simply build a little fix to make it all work like magic.

If the second user can't write the the cache (because it is locked), simply echo the contents of the rendered page, not saving it to the cache.
The first user is dealing with the cache, so just render the output.

With CI, this is how it works...

Line 299 of Output.php:

Code:

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

The cache file is only written if it's 'really_writable' (not locked)

Line 59 of Output.php:

Code:

elseif (($fp = @fopen($file, 'ab')) === FALSE)
    {
        return FALSE;

So... if we cant write it, just render the second users' request.
[/quote]

Untrue.

The second user will wait for the first user to finish writing, and then the second user will overwrite the cache. In fact, on excessively busy sites, many users will stack up waiting to write to the cache. The way to prevent waits and make sure that the second user doesn't write to cache is to use a non-blocking lock in combination with the exclusive lock, but it doesn't work on windows. fopen() won't return false when a file is locked for writing.

Here is some code to demonstrate. Put this method in a controller and open a few browsers each with a different id as a url parameter.

Code:

// Create a file "myfile.txt" in the controllers directory and call
// this method from a few browsers like so:
// http://localhost/controller/test/1, http://localhost/controller/test/2, etc.
function test($id)
{
   $file_name = APPPATH . 'controllers/myfile.txt';

   if ( ! $fp = @fopen($file_name, 'wb'))
   {
       echo('Cannot write to file. Segment id: ' . $id);
   }
   else
   {
      flock( $fp, LOCK_EX );
      sleep(10);
      fwrite( $fp, 'Writing to file. Segment id: ' . $id);
      echo('Successfully wrote file. Segment id: ' . $id);
      flock($fp, LOCK_UN);
      fclose($fp);
   }
}