Hi Stas, On Sat, Mar 16, 2013 at 9:50 PM, Stas Malyshev <smalys...@sugarcrm.com>wrote:
> Hi! > > > Commit: 92430bcf5dd76bf9b12363693efcc471b3527618 > > Author: Dmitry Stogov <dmi...@zend.com> Sat, 16 Mar 2013 > 16:08:11 +0400 > > Parents: ac05e68f5523a2cd1b4972f43188e65c29ef4ae4 > > Branches: PHP-5.5 master > > > > Link: > http://git.php.net/?p=php-src.git;a=commitdiff;h=92430bcf5dd76bf9b12363693efcc471b3527618 > > > > Log: > > Fixed compatibility with ext/phar > > > > Changed paths: > > M ext/opcache/ZendAccelerator.c > > M ext/opcache/zend_accelerator_module.c > > > > > > Diff: > > diff --git a/ext/opcache/ZendAccelerator.c > b/ext/opcache/ZendAccelerator.c > > index 17eac09..daf80ce 100644 > > --- a/ext/opcache/ZendAccelerator.c > > +++ b/ext/opcache/ZendAccelerator.c > > @@ -1221,7 +1221,9 @@ static zend_persistent_script > *compile_and_cache_file(zend_file_handle *file_han > > } > > > > #if ZEND_EXTENSION_API_NO >= PHP_5_3_X_API_NO > > - if (file_handle->type == ZEND_HANDLE_STREAM) { > > + if (file_handle->type == ZEND_HANDLE_STREAM && > > + (!strstr(file_handle->filename, ".phar") || > > + strstr(file_handle->filename, "://"))) { > > char *buf; > > size_t size; > > I think we here are taking too broad approach. This disables O+ for any > file that is loaded via a stream, if I understand it correctly - but > First of all these checks are not going to disable file caching. They are used to disable file preloading using zend_stream_fixup(). It's necessary to preload STREAMS because their reading at compile-time may cause some user callback invocation that may be executed in wrong context. > many streams are just fine, e.g. file://, and many user streams that > don't do weird things but do normal work like reading files, etc. can > probably be compatible. In fact, it's be very good idea to make phar > play nice with O+ - otherwise any phar distro automatically gets > performance penalty. What's the reason phar is excluded? > The test ext/phar/tests/phar_mount.phpt was failed before the fix. Also, the check itself is wrong - it checks for filename which contains > .phar anywhere, so something like /htdocs/moses.vs.pharaoh/index.php > would be wrongly considered a phar file. > This is the inverted check taken from ext/phar/phar.c:3337 phar_compile_file(). I agree it's not accurate. Thanks. Dmitry. > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 >
