Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:40126
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: <48B7BF19.5020007@zend.com>
Date: Fri, 29 Aug 2008 13:19:21 +0400
User-Agent: Thunderbird 2.0.0.16 (X11/20080723)
MIME-Version: 1.0
To: Matt Wilmas <php_lists@realplain.com>
CC: internals@lists.php.net
References: <00ea01c8a160$2edd8160$0201a8c0@pc1> <016c01c8eccd$e28cfac0$0201a8c0@pc1> <488835D0.1040005@zend.com> <00e501c8ed77$3498cf20$0201a8c0@pc1> <48889901.8000008@zend.com> <015101c8f306$34c358f0$0201a8c0@pc1> <4891CC46.2020208@zend.com> <020e01c8f32e$a82a9fb0$0201a8c0@pc1> <4891F189.7030202@zend.com> <00be01c90919$0592d0f0$0201a8c0@pc1>
In-Reply-To: <00be01c90919$0592d0f0$0201a8c0@pc1>
Content-Type: multipart/mixed;
 boundary="------------090002070900080800060004"
Subject: Re: [PHP-DEV] [PATCH] No runtime fetching of built-in global constants
From: dmitry@zend.com (Dmitry Stogov)

--------------090002070900080800060004
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Hi Matt,

I updated your patch a little bit to make it more clear (from my point
of view).
Please take a look.

Thanks. Dmitry.

Matt Wilmas wrote:
> Hi Dmitry,
> 
> Well, it's been awhile since Alpha 1 :-), so I wanted to finally resend this
> before Alpha 2!  I agree that the additional optimization probably wouldn't
> happen often, as there won't be that much namespace usage right away, I
> assume.  But I think it makes sense to handle :: prefix constants, since
> they're known to have global scope, and I can see the future [online]
> optimization tip: "When using namespaces, use :: for global constants to get
> compile-time substitution." ;-)
> 
> The code is the same as before, just updated the patch.  It doesn't really
> seem less clear, to me, than the current code.  In zend_do_fetch_constant(),
> the "check_namespace" variable may be better named something like
> "from_namespace" according to my changes, to be more clear, but I left that
> out to make the patch as simple as possible.
> 
> http://realplain.com/php/ct_const_fixes.diff
> http://realplain.com/php/ct_const_fixes_5_3.diff
> 
> 
> Thanks,
> Matt
> 
> 
> ----- Original Message -----
> From: "Dmitry Stogov"
> Sent: Thursday, July 31, 2008
> 
>> Hi Matt,
>>
>> Now I know. :)
>> Anyway, I don't think this optimization will work often.
>>
>> Send me the patch after Alpha1 release.
>>
>> Thanks. Dmitry.
>>
>>
>> Matt Wilmas wrote:
>>> Hi Dmitry,
>>>
>>> Do you know that with your changes, no substitution will happen in a
>>> namespace even when using :: prefix? :-/  (That's what I would do when I
>>> know it's global, for optimization.)  Or is that what you meant by "not
> so
>>> optimal?"
>>>
>>>
>>> - Matt
>>>
>>>
>>> ----- Original Message -----
>>> From: "Dmitry Stogov"
>>> Sent: Thursday, July 31, 2008
>>>
>>>> Thanks Matt. I committed near the same patch.
>>>> It's not so optimal, but little bit more clear.
>>>>
>>>> Thanks. Dmitry.
>>>>
>>>> Matt Wilmas wrote:
>>>>> Hi Dmitry,
>>>>>
>>>>> For the behavior change that I mentioned in the other thread, with
> this
>>>>> code:
>>>>>
>>>>> function foo() {
>>>>>     static $a = -PHP_INT_MAX;
>>>>> }
>>>>>
>>>>> Which could work sometimes, and sometimes not (if in a namespace or
>>>>> ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION is set).  I changed things so
> that
>>>>> there is no substitution of constants (except with CT_SUBST flag, like
>>>>> always) for compile time constants (ZEND_CT mode).  They don't create
> a
>>>>> FETCH_CONSTANT opcode anyway. :-)
>>>>>
>>>>> Another thing I realized wasn't getting optimized (runtime constants),
>>> which
>>>>> can be, is with:
>>>>>
>>>>> namespace foo;
>>>>> $a = ::PHP_INT_MAX;  // :: for global scope
>>>>>
>>>>> So the patch allows substitution there as well.
>>>>>
>>>>> http://realplain.com/php/ct_const_fixes.diff
>>>>> http://realplain.com/php/ct_const_fixes_5_3.diff
> 

--------------090002070900080800060004
Content-Type: text/plain;
 name="ct_const_fixes_5_3.diff.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="ct_const_fixes_5_3.diff.txt"

Index: Zend/zend_compile.c
===================================================================
RCS file: /repository/ZendEngine2/zend_compile.c,v
retrieving revision 1.647.2.27.2.41.2.84
diff -u -p -d -r1.647.2.27.2.41.2.84 zend_compile.c
--- Zend/zend_compile.c	24 Aug 2008 18:22:33 -0000	1.647.2.27.2.41.2.84
+++ Zend/zend_compile.c	29 Aug 2008 09:09:48 -0000
@@ -1392,7 +1392,7 @@ void zend_do_begin_lambda_function_decla
 	zend_do_begin_function_declaration(function_token, &function_name, 0, return_reference, NULL TSRMLS_CC);
 
 	result->op_type = IS_TMP_VAR;
-	result->u.var = get_temporary_variable(current_op_array);;
+	result->u.var = get_temporary_variable(current_op_array);
 
 	current_op = &current_op_array->opcodes[current_op_number];
 	current_op->opcode = ZEND_DECLARE_LAMBDA_FUNCTION;
@@ -3767,7 +3767,7 @@ void zend_do_end_new_object(znode *resul
 	*result = CG(active_op_array)->opcodes[new_token->u.opline_num].result;
 }
 
-static zend_constant* zend_get_ct_const(const zval *const_name, int mode TSRMLS_DC) /* {{{ */
+static zend_constant* zend_get_ct_const(const zval *const_name, int all_internal_constants_substitution TSRMLS_DC) /* {{{ */
 {
 	zend_constant *c = NULL;
 
@@ -3786,9 +3786,8 @@ static zend_constant* zend_get_ct_const(
 	if (c->flags & CONST_CT_SUBST) {
 		return c;
 	}
-	if (mode == ZEND_RT &&
+	if (all_internal_constants_substitution &&
 	    (c->flags & CONST_PERSISTENT) &&
-	    !CG(current_namespace) &&
 	    !(CG(compiler_options) & ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION) &&
 	    Z_TYPE(c->value) != IS_CONSTANT &&
 	    Z_TYPE(c->value) != IS_CONSTANT_ARRAY) {
@@ -3798,9 +3797,9 @@ static zend_constant* zend_get_ct_const(
 }
 /* }}} */
 
-static int zend_constant_ct_subst(znode *result, zval *const_name, int mode TSRMLS_DC) /* {{{ */
+static int zend_constant_ct_subst(znode *result, zval *const_name, int all_internal_constants_substitution TSRMLS_DC) /* {{{ */
 {
-	zend_constant *c = zend_get_ct_const(const_name, mode TSRMLS_CC);
+	zend_constant *c = zend_get_ct_const(const_name, all_internal_constants_substitution TSRMLS_CC);
 
 	if (c) {
 		zval_dtor(const_name);
@@ -3827,7 +3826,7 @@ void zend_do_fetch_constant(znode *resul
 		zval_dtor(&constant_container->u.constant);
 		check_namespace = 1;
 		constant_container = NULL;
-		fetch_type = ZEND_FETCH_CLASS_RT_NS_CHECK | IS_CONSTANT_RT_NS_CHECK;;
+		fetch_type = ZEND_FETCH_CLASS_RT_NS_CHECK | IS_CONSTANT_RT_NS_CHECK;
 	}
 
 	switch (mode) {
@@ -3843,7 +3842,7 @@ void zend_do_fetch_constant(znode *resul
 				zend_do_build_full_name(NULL, constant_container, constant_name TSRMLS_CC);
 				*result = *constant_container;
 				result->u.constant.type = IS_CONSTANT | fetch_type;
-			} else if (fetch_type || !zend_constant_ct_subst(result, &constant_name->u.constant, ZEND_CT TSRMLS_CC)) {
+			} else if (fetch_type || !zend_constant_ct_subst(result, &constant_name->u.constant, 0 TSRMLS_CC)) {
 				if (check_namespace && CG(current_namespace)) {
 					/* We assume we use constant from the current namespace
 					   if it is not prefixed. */
@@ -3860,7 +3859,7 @@ void zend_do_fetch_constant(znode *resul
 			break;
 		case ZEND_RT:
 			if (constant_container ||
-			    !zend_constant_ct_subst(result, &constant_name->u.constant, ZEND_RT TSRMLS_CC)) {
+			    !zend_constant_ct_subst(result, &constant_name->u.constant, (!CG(current_namespace) || !check_namespace) TSRMLS_CC)) {
 				zend_op *opline;
 
 				if (constant_container) {
@@ -5166,7 +5165,7 @@ void zend_do_declare_constant(znode *nam
 		zend_error(E_COMPILE_ERROR, "Arrays are not allowed as constants");
 	}
 
-	if (zend_get_ct_const(&name->u.constant, ZEND_CT TSRMLS_CC)) {
+	if (zend_get_ct_const(&name->u.constant, 0 TSRMLS_CC)) {
 		zend_error(E_COMPILE_ERROR, "Cannot redeclare constant '%s'", Z_STRVAL(name->u.constant));
 	}
 

--------------090002070900080800060004--