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

From: Date: Mon, 13 Mar 2006 22:18:05 +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
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


Index: Zend/zend_hash.c =================================================================== RCS file: /repository/ZendEngine2/zend_hash.c,v retrieving revision 1.134 diff -u -p -d -r1.134 zend_hash.c --- Zend/zend_hash.c 26 Feb 2006 11:57:14 -0000 1.134 +++ Zend/zend_hash.c 13 Mar 2006 19:29:30 -0000 @@ -749,13 +749,14 @@ ZEND_API void zend_hash_graceful_reverse ZEND_API void zend_hash_apply(HashTable *ht, apply_func_t apply_func TSRMLS_DC) { Bucket *p; + int result = ZEND_HASH_APPLY_KEEP; IS_CONSISTENT(ht); HASH_PROTECT_RECURSION(ht); p = ht->pListHead; - while (p != NULL) { - if (apply_func(p->pData TSRMLS_CC)) { + while (p != NULL && !(result & ZEND_HASH_APPLY_STOP)) { + if ((result = apply_func(p->pData TSRMLS_CC)) & ZEND_HASH_APPLY_REMOVE) { p = zend_hash_apply_deleter(ht, p); } else { p = p->pListNext; @@ -768,13 +769,14 @@ ZEND_API void zend_hash_apply(HashTable ZEND_API void zend_hash_apply_with_argument(HashTable *ht, apply_func_arg_t apply_func, void *argument TSRMLS_DC) { Bucket *p; + int result = ZEND_HASH_APPLY_KEEP; IS_CONSISTENT(ht); HASH_PROTECT_RECURSION(ht); p = ht->pListHead; - while (p != NULL) { - if (apply_func(p->pData, argument TSRMLS_CC)) { + while (p != NULL && !(result & ZEND_HASH_APPLY_STOP)) { + if ((result = apply_func(p->pData, argument TSRMLS_CC)) & ZEND_HASH_APPLY_REMOVE) { p = zend_hash_apply_deleter(ht, p); } else { p = p->pListNext; @@ -784,24 +786,25 @@ ZEND_API void zend_hash_apply_with_argum } -ZEND_API void zend_hash_apply_with_arguments(HashTable *ht, apply_func_args_t destruct, int num_args, ...) +ZEND_API void zend_hash_apply_with_arguments(HashTable *ht, apply_func_args_t apply_func, int num_args, ...) { Bucket *p; va_list args; zend_hash_key hash_key; + int result = ZEND_HASH_APPLY_KEEP; IS_CONSISTENT(ht); HASH_PROTECT_RECURSION(ht); p = ht->pListHead; - while (p != NULL) { + while (p != NULL && !(result & ZEND_HASH_APPLY_STOP)) { va_start(args, num_args); hash_key.nKeyLength = p->nKeyLength; hash_key.h = p->h; hash_key.type = p->key.type; hash_key.arKey.s = p->key.arKey.s; - if (destruct(p->pData, num_args, args, &hash_key)) { + if ((result = apply_func(p->pData, num_args, args, &hash_key)) & ZEND_HASH_APPLY_REMOVE) { p = zend_hash_apply_deleter(ht, p); } else { p = p->pListNext;

Thread (6 messages)

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