Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36126
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 85.214.94.56 as permitted sender)
Date: Thu, 13 Mar 2008 19:27:27 +0100
Reply-To: Marcus Boerger <helly@php.net>
Message-ID: <61361416.20080313192727@marcus-boerger.de>
To: "Dmitry Stogov" <dmitry@zend.com>
CC: "Andi Gutmans" <andi@zend.com>, "Stas Malyshev" <stas@zend.com>, 
	"phpxcache" <phpxcache@gmail.com>, <internals@lists.php.net>
In-Reply-To: <06B0D32C7A96544490D18AF653D6BDE5026B9745@il-ex1.zend.net>
References: <06B0D32C7A96544490D18AF653D6BDE5026B9745@il-ex1.zend.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] Patch for opcode caches
From: helly@php.net (Marcus Boerger)

Hello Dmitry,

Monday, March 10, 2008, 1:48:05 PM, you wrote:

> Hi Marcus,

>> > -----Original Message-----
>> > From: Marcus Boerger [mailto:helly@php.net]
>> > Sent: Sunday, March 09, 2008 3:00 PM
>> > To: Dmitry Stogov
>> > Cc: Derick Rethans; internals Mailing List; Andi Gutmans; Stas 
>> > Malyshev; phpxcache
>> > Subject: Re: [PHP-DEV] Patch for opcode caches
>> > 
>> > Hello Dmitry,
>> > 
>> > please don't apply. The patch looks rather rough and untested (see 
>> > below).

> It was tested at least with Zend products and xcache.

Good, I am just worried that we introduce different behavior. We should
just have one execution mode. Your change is better now having a more
detailed view on what it does. But I think we should not have the old model
as well.

>> >  Also I really disagree to making the engine even 
>> > more complex 
>> > and adding
>> > even more different behavior ways. That way we just introduce more
>> > errors
>> > as we cannot test the engine in all its modes.

> It is not a good argument to hide this from APC, eAccelerator and
> xcache.

>> >  We simply do not have
>> > the
>> > infrastructure for that. And that means we will add even more bugs.

> You will able to test it with "make test" and new versions of
> APC/eAccelerator/XCache ...
> At least I tested it in this way without problems

But the majority of people cannot unless we bundle one of those. So all we
normal people get is an increased ability of errors.

> 2) Changes related to ZEND_ACC_IMPLEMENT_INTERFACES  fix unnecessary
> zend_verify_abstarct_class() calls. (For some reason it might be called
> twice for the same class)

good catch then

> 3)  ZEND_COMPILE_OPTION_... is to long, but I can accept this and of
> couse I'll add comments as you requested.

good :-)

> 4) zend_vm_opcodes.h is autogenerated file, zend_vm_gen.php chenged the
> indents  :)

ok

> 5) I'll recheck the -e option. Probably I missied something there.

I did not see -e in the list of allowed flags. So it cannot work.

Best regards,
 Marcus