Hi Marcus,
Thank you for catching me.
I'll fix this today.
Dmitry.
> -----Original Message-----
> From: Marcus Boerger [mailto:[email protected]]
> Sent: Tuesday, March 14, 2006 1:18 AM
> To: Dmitry Stogov
> Cc: [email protected]
> Subject: Re: [ZEND-ENGINE-CVS] cvs: ZendEngine2(PHP_5_1) /
> zend_compile.h zend_constants.c zend_execute_API.c zend_opcode.c
>
>
> Hello Dmitry,
>
> you didn't change the apply functions to respect the
> ZEND_HASH_APPLY_* consts, i did so now for HEAD. For 5.1 we
> have it inconsistent now. And changing it would mean a BC break.
>
> As a consequence i can no longer use run-tests.php in HEAD
> but something else seems to be wrong, too.
>
> A patch that fixed the hash apply functions is applied.
>
> best regards
> marcus
>
> Monday, March 13, 2006, 12:13:42 PM, you wrote:
>
> > dmitry Mon Mar 13 11:13:42 2006 UTC
>
> > Modified files: (Branch: PHP_5_1)
> > /ZendEngine2 zend_compile.h zend_constants.c
> zend_execute_API.c
> > zend_opcode.c
> > Log:
> > Optimized cleanup loops on request shutdown
> >
> >
> >
> http://cvs.php.net/viewcvs.cgi/ZendEngine2/zend_compile.h?r1=1.316.2.7
> > &r2=1.316.2.8&diff_format=u
> > Index: ZendEngine2/zend_compile.h
> > diff -u ZendEngine2/zend_compile.h:1.316.2.7
> ZendEngine2/zend_compile.h:1.316.2.8
> > --- ZendEngine2/zend_compile.h:1.316.2.7 Sat Feb 25
> 18:25:44 2006
> > +++ ZendEngine2/zend_compile.h Mon Mar 13 11:13:42 2006
> > @@ -17,7 +17,7 @@
> >
> >
> +-------------------------------------------------------------
> ---------+
> > */
> >
> > -/* $Id: zend_compile.h,v 1.316.2.7 2006/02/25 18:25:44
> helly Exp $ */
> > +/* $Id: zend_compile.h,v 1.316.2.8 2006/03/13 11:13:42
> dmitry Exp $
> > +*/
> >
> > #ifndef ZEND_COMPILE_H
> > #define ZEND_COMPILE_H
> > @@ -517,6 +517,7 @@
> > ZEND_API void zend_file_handle_dtor(zend_file_handle *fh);
> ZEND_API
> > int zend_cleanup_class_data(zend_class_entry **pce TSRMLS_DC);
> > ZEND_API int zend_cleanup_function_data(zend_function *function
> > TSRMLS_DC);
> > +ZEND_API int zend_cleanup_function_data_full(zend_function
> *function
> > +TSRMLS_DC);
> >
> > ZEND_API void destroy_zend_function(zend_function *function
> > TSRMLS_DC); ZEND_API void zend_function_dtor(zend_function
> > *function);
> >
> http://cvs.php.net/viewcvs.cgi/ZendEngine2/zend_constants.c?r1
> =1.71.2.1&r2=1.71.2.2&diff_format=u
> > Index: ZendEngine2/zend_constants.c
> > diff -u ZendEngine2/zend_constants.c:1.71.2.1
> > ZendEngine2/zend_constants.c:1.71.2.2
> > --- ZendEngine2/zend_constants.c:1.71.2.1 Wed Jan 4
> 23:53:04 2006
> > +++ ZendEngine2/zend_constants.c Mon Mar 13 11:13:42 2006
> > @@ -17,7 +17,7 @@
> >
> >
> +-------------------------------------------------------------
> ---------+
> > */
> >
> > -/* $Id: zend_constants.c,v 1.71.2.1 2006/01/04 23:53:04
> andi Exp $ */
> > +/* $Id: zend_constants.c,v 1.71.2.2 2006/03/13 11:13:42
> dmitry Exp $
> > +*/
> >
> > #include "zend.h"
> > #include "zend_constants.h"
> > @@ -55,11 +55,13 @@
> >
> > static int clean_non_persistent_constant(zend_constant *c
> TSRMLS_DC)
> > {
> > - if (c->flags & CONST_PERSISTENT) {
> > - return EG(full_tables_cleanup) ? 0 :
> ZEND_HASH_APPLY_STOP;
> > - } else {
> > - return EG(full_tables_cleanup) ? 1 :
> ZEND_HASH_APPLY_REMOVE;
> > - }
> > + return (c->flags & CONST_PERSISTENT) ?
> ZEND_HASH_APPLY_STOP :
> > + ZEND_HASH_APPLY_REMOVE;
>
> I am quite sure stop is wrong here and eitehr way we don't
> respect the consts in all apply functions yet.
>
> > +}
> > +
> > +
> > +static int clean_non_persistent_constant_full(zend_constant *c
> > +TSRMLS_DC) {
> > + return (c->flags & CONST_PERSISTENT) ?
> ZEND_HASH_APPLY_KEEP :
> > +ZEND_HASH_APPLY_REMOVE;
> > }
> >
> >
> > @@ -153,7 +155,7 @@
> > void clean_non_persistent_constants(TSRMLS_D)
> > {
> > if (EG(full_tables_cleanup)) {
> > - zend_hash_apply(EG(zend_constants), (apply_func_t)
> > clean_non_persistent_constant TSRMLS_CC);
> > + zend_hash_apply(EG(zend_constants), (apply_func_t)
> > clean_non_persistent_constant_full TSRMLS_CC);
> > } else {
> > zend_hash_reverse_apply(EG(zend_constants),
> > (apply_func_t) clean_non_persistent_constant TSRMLS_CC);
> > }
> >
> http://cvs.php.net/viewcvs.cgi/ZendEngine2/zend_execute_API.c?r1=1.331
> > .2.16&r2=1.331.2.17&diff_format=u
> > Index: ZendEngine2/zend_execute_API.c
> > diff -u ZendEngine2/zend_execute_API.c:1.331.2.16
> > ZendEngine2/zend_execute_API.c:1.331.2.17
> > --- ZendEngine2/zend_execute_API.c:1.331.2.16 Fri Mar 3
> 12:06:29 2006
> > +++ ZendEngine2/zend_execute_API.c Mon Mar 13 11:13:42 2006
> > @@ -17,7 +17,7 @@
> >
> >
> +-------------------------------------------------------------
> ---------+
> > */
> >
> > -/* $Id: zend_execute_API.c,v 1.331.2.16 2006/03/03 12:06:29 dmitry
> > Exp $ */
> > +/* $Id: zend_execute_API.c,v 1.331.2.17 2006/03/13 11:13:42 dmitry
> > +Exp $ */
> >
> > #include <stdio.h>
> > #include <signal.h>
> > @@ -95,23 +95,27 @@
> > }
> >
> >
> > -static int is_not_internal_function(zend_function *function
> > TSRMLS_DC)
> > +static int
>
> > (zend_function *function TSRMLS_DC)
> > {
> > - if (function->type == ZEND_INTERNAL_FUNCTION) {
> > - return EG(full_tables_cleanup) ? 0 :
> ZEND_HASH_APPLY_STOP;
> > - } else {
> > - return EG(full_tables_cleanup) ? 1 :
> ZEND_HASH_APPLY_REMOVE;
> > - }
> > + return (function->type == ZEND_INTERNAL_FUNCTION) ?
> > ZEND_HASH_APPLY_STOP : ZEND_HASH_APPLY_REMOVE;
>
> same here, stop is probably wrong
>
> > }
> >
> >
> > -static int is_not_internal_class(zend_class_entry **ce TSRMLS_DC)
> > +static int
> clean_non_persistent_function_full(zend_function *function
> > +TSRMLS_DC)
> > {
> > - if ((*ce)->type == ZEND_INTERNAL_CLASS) {
> > - return EG(full_tables_cleanup) ? 0 :
> ZEND_HASH_APPLY_STOP;
> > - } else {
> > - return EG(full_tables_cleanup) ? 1 :
> ZEND_HASH_APPLY_REMOVE;
> > - }
> > + return (function->type == ZEND_INTERNAL_FUNCTION) ?
> > ZEND_HASH_APPLY_KEEP : ZEND_HASH_APPLY_REMOVE;
> > +}
> > +
> > +
> > +static int clean_non_persistent_class(zend_class_entry **ce
> > +TSRMLS_DC) {
> > + return ((*ce)->type == ZEND_INTERNAL_CLASS) ?
> > ZEND_HASH_APPLY_STOP : ZEND_HASH_APPLY_REMOVE;
>
> Once more
>
> > +}
> > +
> > +
> > +static int clean_non_persistent_class_full(zend_class_entry **ce
> > +TSRMLS_DC) {
> > + return ((*ce)->type == ZEND_INTERNAL_CLASS) ?
> > ZEND_HASH_APPLY_KEEP : ZEND_HASH_APPLY_REMOVE;
> > }
> >
> >
> > @@ -251,18 +255,22 @@
> > So we want first of all to clean up all
> data and then move to tables destruction.
> > Note that only run-time accessed data
> need to be cleaned up, pre-defined data can
> > not contain objects and thus are not
> probelmatic */
> > - zend_hash_apply(EG(function_table), (apply_func_t)
> > zend_cleanup_function_data TSRMLS_CC);
> > + if (EG(full_tables_cleanup)) {
> > + zend_hash_apply(EG(function_table),
> > (apply_func_t) zend_cleanup_function_data_full TSRMLS_CC);
> > + } else {
> > + zend_hash_reverse_apply(EG(function_table),
> > (apply_func_t) zend_cleanup_function_data TSRMLS_CC);
> > + }
> > zend_hash_apply(EG(class_table), (apply_func_t)
> > zend_cleanup_class_data TSRMLS_CC);
> >
> > zend_ptr_stack_destroy(&EG(argument_stack));
> >
> > /* Destroy all op arrays */
> > if (EG(full_tables_cleanup)) {
> > - zend_hash_apply(EG(function_table),
> > (apply_func_t) is_not_internal_function TSRMLS_CC);
> > - zend_hash_apply(EG(class_table),
> (apply_func_t) is_not_internal_class TSRMLS_CC);
> > + zend_hash_apply(EG(function_table),
> > (apply_func_t) clean_non_persistent_function_full TSRMLS_CC);
> > + zend_hash_apply(EG(class_table),
> > + (apply_func_t)
> > clean_non_persistent_class_full TSRMLS_CC);
> > } else {
> > - zend_hash_reverse_apply(EG(function_table),
> > (apply_func_t) is_not_internal_function TSRMLS_CC);
> > - zend_hash_reverse_apply(EG(class_table),
> > (apply_func_t) is_not_internal_class TSRMLS_CC);
> > + zend_hash_reverse_apply(EG(function_table),
> > (apply_func_t) clean_non_persistent_function TSRMLS_CC);
> > + zend_hash_reverse_apply(EG(class_table),
> > (apply_func_t) clean_non_persistent_class TSRMLS_CC);
> > }
> >
> > while
> (EG(symtable_cache_ptr)>=EG(symtable_cache)) {
> >
> http://cvs.php.net/viewcvs.cgi/ZendEngine2/zend_opcode.c?r1=1.110.2.3&
> > r2=1.110.2.4&diff_format=u
> > Index: ZendEngine2/zend_opcode.c
> > diff -u ZendEngine2/zend_opcode.c:1.110.2.3
> ZendEngine2/zend_opcode.c:1.110.2.4
> > --- ZendEngine2/zend_opcode.c:1.110.2.3 Wed Jan 4 23:53:04 2006
> > +++ ZendEngine2/zend_opcode.c Mon Mar 13 11:13:42 2006
> > @@ -17,7 +17,7 @@
> >
> >
> +-------------------------------------------------------------
> ---------+
> > */
> >
> > -/* $Id: zend_opcode.c,v 1.110.2.3 2006/01/04 23:53:04 andi Exp $ */
> > +/* $Id: zend_opcode.c,v 1.110.2.4 2006/03/13 11:13:42
> dmitry Exp $ */
> >
> > #include <stdio.h>
> >
> > @@ -132,8 +132,18 @@
> > {
> > if (function->type == ZEND_USER_FUNCTION) {
> > zend_cleanup_op_array_data((zend_op_array
> *) function);
> > - }
> > - return 0;
> > + return ZEND_HASH_APPLY_KEEP;
> > + } else {
> > + return ZEND_HASH_APPLY_STOP;
> > + }
> > +}
> > +
> > +ZEND_API int zend_cleanup_function_data_full(zend_function
> *function
> > +TSRMLS_DC) {
> > + if (function->type == ZEND_USER_FUNCTION) {
> > + zend_cleanup_op_array_data((zend_op_array
> *) function);
> > + }
> > + return ZEND_HASH_APPLY_KEEP;
> > }
> >
> > ZEND_API int zend_cleanup_class_data(zend_class_entry **pce
> > TSRMLS_DC) @@ -142,7 +152,7 @@
> > /* Clean all parts that can contain run-time data */
> > /* Note that only run-time accessed data
> need to be cleaned up, pre-defined data can
> > not contain objects and thus are not
> probelmatic */
> > - zend_hash_apply(&(*pce)->function_table,
> (apply_func_t)
> > zend_cleanup_function_data TSRMLS_CC);
> > + zend_hash_apply(&(*pce)->function_table,
> > + (apply_func_t)
> > zend_cleanup_function_data_full TSRMLS_CC);
> > (*pce)->static_members = NULL;
> > } else if (CE_STATIC_MEMBERS(*pce)) {
> > zend_hash_destroy(CE_STATIC_MEMBERS(*pce));
>
>
>
>
> Best regards,
> Marcus
>