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;