Added function to check if OPCache already has a script cached

[Danack]

The PR is to be able to check to see if OPCache already has a script cached - as it's useful information for autoloaders.

For some class autoloaders (including Composer) it is possible for a script to exist in one of several possible directores. To load that file the autoloader needs to call file_exists e.g. from the composer autoloader:

foreach ($this->prefixes[$first] as $prefix => $dirs) {
    if (0 === strpos($class, $prefix)) {
        foreach ($dirs as $dir) {
            if (file_exists($dir . DIRECTORY_SEPARATOR . $classPath)) {
                return $dir . DIRECTORY_SEPARATOR . $classPath;
            }
        }
    }
}

As PHP doesn't cache the result when the file doesn't exist, this can easily cause several hits to the file system.

Adding the function opcache_script_cached allows the autoloader to check to see if OPCache has already loaded the file in all of the possible locations that where a script could exist, before hitting the filesystem.

foreach ($this->prefixes[$first] as $prefix => $dirs) {
    //Check all possible paths in OPCache before checking the file system
    if (0 === strpos($class, $prefix)) {
        foreach ($dirs as $dir) {
            $filename = $dir.DIRECTORY_SEPARATOR.$classPath;
            if (opcache_script_cached($filename) == true) {
                return $filename;
            }
        }

        foreach ($dirs as $dir) {
            $filename = $dir.DIRECTORY_SEPARATOR.$classPath;
            if (file_exists($filename)) {
                return $filename;
            }
        }
    }
}

i.e. you can use the information from opcache_script_cached to avoid any filesystem hit, once the script is loaded into OPCache.

Although this information is retrievable through opcache_get_status the user has to do a lot more work to use it, searching through the large array returned, and manually correcting the paths.

[TerryE]

You actually omit the main justification for adding this API call.

The "the user has to do a lot more work to do to use opcache_get_status()" isn't compelling -- its a few extra lines of PHP with small runtime cost compare to a file_exists().

The main reason is that a subset of API calls are subject to the opcache.restrict_api filter so that their use can be restricted to sysadmins, because they either allow the caller to impact the global cache or have global knowledge, and so these include opcache_get_status(). There is a strong case for giving open access to your call because (i) it is readonly and (ii) it returns a simple boolean based and a filename supplied. However:

• Your routine returns a bool so it would make a lot more sense for it to be called opcache_is_script_cached().
• If you are going to bypass validate_api_restriction() then you should mirror the structure of ZEND_FUNCTION(opcache_compile_file).
• So zend_parse_parameters() should only do a return; on failure (in fact it does a bailout so never returns, but the pattern should be consistent with user uses.)
• Your line 712 seems random; it makes no sense to me. Mirror the compile file test in lines 758-761. Though again there is an argument for not raising an error here, but simply returning FALSE and documenting a FALSE return simply either the file isn't cached or OPcache isn't enabled.

Other than these points, I would +1 a corrected pull request.

[Danack]

Thanks for the feedback - fyi I took the check for whether OPCache is running from line 420 in zend_accel_info. It made no sense to me either, so have changed to your suggestion.

I added the validate_api_restriction() check as, yes, I can see people could object to it being a source of information leaking.

[Danack]

Actually do you think the function should be protected by validate_api_restriction or not?

The "the user has to do a lot more work to do to use opcache_get_status()" isn't compelling -- its a few extra lines of PHP with small runtime cost compare to a file_exists()

Although it would almost certainly be quicker than doing a file_exists, it's not a non-trivial amount of data to search through either.

Because a large chunk of the file path will be the same for every script e.g. /home/websiteName/live/vendor/companyName/projectName/src/companyName/projectName/, for a large site with 2000 scripts in the cache, and need to load 400 scripts for a page, that would be a lot of string compare to do, as well as a lot of stepping through arrays in slow userland code.

Yes, almost certainly faster than doing a file_exists but not nearly as fast as doing the check in internal C code.

[TerryE]

No, I feell that it falls into the same category as opcache_get_status() -- that is this is a user function rather than a sysadmin function and therefore the api_restriction check should not apply. However I am only a contributor and not part of the core Zend dev team.

[dstogov]

merged into PHP-5.5 and this repo