Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:39369
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.163 as permitted sender)
Message-ID: <488C5464.1030706@zend.com>
Date: Sun, 27 Jul 2008 14:56:36 +0400
User-Agent: Thunderbird 2.0.0.16 (Windows/20080708)
MIME-Version: 1.0
To: Matt Wilmas <php_lists@realplain.com>
CC: internals@lists.php.net, Andi Gutmans <andi@zend.com>, 
 Stanislav Malyshev <stas@zend.com>
References: <00ea01c8a160$2edd8160$0201a8c0@pc1> <016c01c8eccd$e28cfac0$0201a8c0@pc1> <488835D0.1040005@zend.com> <00e501c8ed77$3498cf20$0201a8c0@pc1> <48889901.8000008@zend.com> <022c01c8ee74$5b539340$0201a8c0@pc1>
In-Reply-To: <022c01c8ee74$5b539340$0201a8c0@pc1>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: New optimization idea; was: No runtime fetching of built-in global
 constants
From: dmitry@zend.com (Dmitry Stogov)

Hi Matt,

At first as you are a scanner expert, I would like you to look into 
another optimization idea.

Probably for historical reason PHP supports shebang lines
(#! /usr/bin/php) on top of php files. Especially to handle them PHP 
(CGI/FastCGI/CLI) opens file and check for it. So even with opcode 
caches FastCGI PHP does open syscall for the requested script, however 
with opcode caches it's absolutely useless.

In case PHP scanner will handle shebang lines itself, we will able to 
save this syscall.

I never had time and enough flex/re2c knowledge to implement this idea 
myself. May be you'll able to look into the problem. In case you find a 
simple solution we will able to do it in php-5.3.

Most PHP hosters and large sites use FastCGI with opcode caches (it is 
also the primary way for MS Windows users), so this optimization is 
really important.

[see below]

Matt Wilmas wrote:
> Hi Dmitry,
> 
> I saw that you commited this patch, with the addition of only replacing
> persistent constants (just mentioning for others on the list).  The attached
> patches have a few tweaks:
> 
> The main thing I noticed is that if "something" creates a persistent,
> case-INsensitive constant (any of those in "standard" PHP, besides
> TRUE/FALSE/NULL?), it could still wrongly be substituted.  My change
> eliminates that possibility.

After yesterday's subbotnik I'm so stupid so cannot understand simple 
tings. :)
Could you point me into the exact part of the patch.

> Checking Z_TYPE(c->value) != IS_CONSTANT[_ARRAY] isn't needed with the
> persistent check now, is it?

I think you are right, but it's better to have this checks, because 
nobody prohibit to create array constants in extensions.

> From orginal patch (zend_constants.c part), the memcmp(...) != 0 isn't
> needed as it will always be true.  If it wasn't, the *first* hash lookup
> wouldn't have failed. :-)  Like I said in the original message, it's old
> code from a long time ago, but was never removed...

I'll check it with more clear head.

Thanks. Dmitry.

> 
> - Matt
> 
> 
> ----- Original Message -----
> From: "Dmitry Stogov"
> Sent: Thursday, July 24, 2008
> 
>> I would propose the attached patch for this optimization.
>>
>> Opcode caches and encoders will have to disable this optimization with
>> ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION
>>
>> Any objections?
>>
>> Thanks. Dmitry.
>>
> 
> 
> ----------------------------------------------------------------------------
> ----
> 
> 
>> Index: Zend/zend_compile.c
>> ===================================================================
>> RCS file: /repository/ZendEngine2/zend_compile.c,v
>> retrieving revision 1.647.2.27.2.41.2.74
>> diff -u -p -d -r1.647.2.27.2.41.2.74 zend_compile.c
>> --- Zend/zend_compile.c 24 Jul 2008 11:47:49 -0000 1.647.2.27.2.41.2.74
>> +++ Zend/zend_compile.c 24 Jul 2008 14:40:12 -0000
>> @@ -3804,6 +3804,12 @@ static zend_constant* zend_get_ct_const(
>>   if (c->flags & CONST_CT_SUBST) {
>>   return c;
>>   }
>> + if (!CG(current_namespace) &&
>> +     !(CG(compiler_options) & ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION) &&
>> +     Z_TYPE(c->value) != IS_CONSTANT &&
>> +     Z_TYPE(c->value) != IS_CONSTANT_ARRAY) {
>> + return c;
>> + }
>>   return NULL;
>>  }
>>  /* }}} */
>> @@ -5169,12 +5175,14 @@ void zend_do_use(znode *ns_name, znode *
>>  void zend_do_declare_constant(znode *name, znode *value TSRMLS_DC) /*
> {{{ */
>>  {
>>   zend_op *opline;
>> + zend_constant *c;
>>
>>   if(Z_TYPE(value->u.constant) == IS_CONSTANT_ARRAY) {
>>   zend_error(E_COMPILE_ERROR, "Arrays are not allowed as constants");
>>   }
>>
>> - if (zend_get_ct_const(&name->u.constant TSRMLS_CC)) {
>> + c = zend_get_ct_const(&name->u.constant TSRMLS_CC);
>> + if (c && (c->flags & CONST_CT_SUBST)) {
>>   zend_error(E_COMPILE_ERROR, "Cannot redeclare constant '%s'",
> Z_STRVAL(name->u.constant));
>>   }
>>
>> Index: Zend/zend_compile.h
>> ===================================================================
>> RCS file: /repository/ZendEngine2/zend_compile.h,v
>> retrieving revision 1.316.2.8.2.12.2.27
>> diff -u -p -d -r1.316.2.8.2.12.2.27 zend_compile.h
>> --- Zend/zend_compile.h 24 Jul 2008 11:47:49 -0000 1.316.2.8.2.12.2.27
>> +++ Zend/zend_compile.h 24 Jul 2008 14:40:13 -0000
>> @@ -762,6 +762,9 @@ END_EXTERN_C()
>>  /* generate ZEND_DECLARE_INHERITED_CLASS_DELAYED opcode to delay early
> binding */
>>  #define ZEND_COMPILE_DELAYED_BINDING (1<<4)
>>
>> +/* disable constant substitution at compile-time */
>> +#define ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION (1<<5)
>> +
>>  /* The default value for CG(compiler_options) */
>>  #define ZEND_COMPILE_DEFAULT ZEND_COMPILE_HANDLE_OP_ARRAY
>>
>>