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. Checking Z_TYPE(c->value) != IS_CONSTANT[_ARRAY] isn't needed with the persistent check now, is it? >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... - 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 > >
Index: Zend/zend_compile.c =================================================================== RCS file: /repository/ZendEngine2/zend_compile.c,v retrieving revision 1.832 diff -u -r1.832 zend_compile.c --- Zend/zend_compile.c 25 Jul 2008 04:54:56 -0000 1.832 +++ Zend/zend_compile.c 25 Jul 2008 09:30:00 -0000 @@ -3996,24 +3996,20 @@ zstr lookup_name = zend_u_str_case_fold(Z_TYPE_P(const_name), Z_UNIVAL_P(const_name), Z_UNILEN_P(const_name), 1, &lookup_name_len); if (zend_u_hash_find(EG(zend_constants), Z_TYPE_P(const_name), lookup_name, lookup_name_len+1, (void **) &c)==SUCCESS) { - if ((c->flags & CONST_CS) && memcmp(c->name.v, Z_UNIVAL_P(const_name).v, UG(unicode)?UBYTES(Z_USTRLEN_P(const_name)):Z_STRLEN_P(const_name))!=0) { + if ((c->flags & CONST_CT_SUBST) && !(c->flags & CONST_CS)) { efree(lookup_name.v); - return NULL; + return c; } - } else { - efree(lookup_name.v); - return NULL; } efree(lookup_name.v); + return NULL; } if (c->flags & CONST_CT_SUBST) { return c; } if ((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) { + !(CG(compiler_options) & ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION)) { return c; } return NULL; Index: Zend/zend_constants.c =================================================================== RCS file: /repository/ZendEngine2/zend_constants.c,v retrieving revision 1.111 diff -u -r1.111 zend_constants.c --- Zend/zend_constants.c 21 Jul 2008 09:36:21 -0000 1.111 +++ Zend/zend_constants.c 25 Jul 2008 09:34:30 -0000 @@ -278,7 +278,7 @@ lookup_name = zend_u_str_case_fold(type, name, name_len, 1, &lookup_name_len); if (zend_u_hash_find(EG(zend_constants), type, lookup_name, lookup_name_len+1, (void **) &c)==SUCCESS) { - if ((c->flags & CONST_CS) && memcmp(c->name.v, name.v, UG(unicode)?UBYTES(name_len):name_len)!=0) { + if (c->flags & CONST_CS) { retval=0; } } else {
Index: Zend/zend_compile.c =================================================================== RCS file: /repository/ZendEngine2/zend_compile.c,v retrieving revision 1.647.2.27.2.41.2.77 diff -u -r1.647.2.27.2.41.2.77 zend_compile.c --- Zend/zend_compile.c 25 Jul 2008 04:54:08 -0000 1.647.2.27.2.41.2.77 +++ Zend/zend_compile.c 25 Jul 2008 09:30:00 -0000 @@ -3791,24 +3791,20 @@ char *lookup_name = zend_str_tolower_dup(Z_STRVAL_P(const_name), Z_STRLEN_P(const_name)); if (zend_hash_find(EG(zend_constants), lookup_name, Z_STRLEN_P(const_name)+1, (void **) &c)==SUCCESS) { - if ((c->flags & CONST_CS) && memcmp(c->name, Z_STRVAL_P(const_name), Z_STRLEN_P(const_name))!=0) { + if ((c->flags & CONST_CT_SUBST) && !(c->flags & CONST_CS)) { efree(lookup_name); - return NULL; + return c; } - } else { - efree(lookup_name); - return NULL; } efree(lookup_name); + return NULL; } if (c->flags & CONST_CT_SUBST) { return c; } if ((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) { + !(CG(compiler_options) & ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION)) { return c; } return NULL; Index: Zend/zend_constants.c =================================================================== RCS file: /repository/ZendEngine2/zend_constants.c,v retrieving revision 1.71.2.5.2.7.2.10 diff -u -r1.71.2.5.2.7.2.10 zend_constants.c --- Zend/zend_constants.c 21 Jul 2008 09:41:00 -0000 1.71.2.5.2.7.2.10 +++ Zend/zend_constants.c 25 Jul 2008 09:34:30 -0000 @@ -231,7 +231,7 @@ lookup_name = zend_str_tolower_dup(name, name_len); if (zend_hash_find(EG(zend_constants), lookup_name, name_len+1, (void **) &c)==SUCCESS) { - if ((c->flags & CONST_CS) && memcmp(c->name, name, name_len) != 0) { + if (c->flags & CONST_CS) { retval=0; } } else {
-- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php