RE: [ZEND-ENGINE-CVS] cvs: ZendEngine2(PHP_5_1) / zend_compile.h zend_constants.c zend_execute_API.c zend_opcode.c

From: Date: Tue, 14 Mar 2006 11:32:59 +0000
Subject: RE: [ZEND-ENGINE-CVS] cvs: ZendEngine2(PHP_5_1) / zend_compile.h zend_constants.c zend_execute_API.c zend_opcode.c
References: 1  Groups: php.zend-engine.cvs 
Request: Send a blank email to [email protected] to get a copy of this message
Hi Marcs,

> Hello Dmitry, Ilia,
> 
>   in addition to the earlier mail here is the patch against 
> 5.1. And in contrast to my HEAD checkout it works pretty fine 
> with it. I ran the tests and got no new failures. I also 
> started to run gcov tests and the first tests didn't show new 
> failures either.

I think this patch makes full sense, but if ZEND_HASH_APPLY_STOP will never
used it will bring just slowdown. :(

> Another thing Dmitry. You optimized apply calls that are used 
> for destruction of hashes. It would be much faster to change 
> them to hash_destroy since that doesn't fix the hash table 
> consistency after every member that gets destructed and reoved.

What table do you mean?
We don't destroy class/function/constant tables on request shutdown.

Thanks. Dmitry.

> regards
> marcus
> 
> 
> Monday, March 13, 2006, 11:18:05 PM, you wrote:
> 
> > 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.33
> >> 1.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
> 


Thread (6 messages)

« previous php.zend-engine.cvs (#4725) next »