#47815 [Opn]: Compile time computation of hash value decrease hash lookup time.

[email protected] ("basant dot kukreja at gmail dot com")
Newsgroups php.bugs
Message-ID <[email protected]>
 ID:               47815
 User updated by:  basant dot kukreja at gmail dot com
 Reported By:      basant dot kukreja at gmail dot com
 Status:           Open
 Bug Type:         Performance problem
 Operating System: Solaris 10
 PHP Version:      5.2.9
 New Comment:

diff -r 00438f7eebe4 php-5.2.9RC3/Zend/zend_compile.h
--- a/php-5.2.9RC3/Zend/zend_compile.h	Tue Mar 17 11:27:02 2009 -0700
+++ b/php-5.2.9RC3/Zend/zend_compile.h	Fri Mar 27 10:18:13 2009 -0700
@@ -83,6 +83,7 @@
 	znode op1;
 	znode op2;
 	ulong extended_value;
+	ulong hval;
 	uint lineno;
 	zend_uchar opcode;
 };
diff -r 00438f7eebe4 php-5.2.9RC3/Zend/zend_execute.c
--- a/php-5.2.9RC3/Zend/zend_execute.c	Tue Mar 17 11:27:02 2009 -0700
+++ b/php-5.2.9RC3/Zend/zend_execute.c	Fri Mar 27 10:18:13 2009 -0700
@@ -930,11 +930,12 @@
 	return NULL;
 }
 
-static inline zval **zend_fetch_dimension_address_inner(HashTable *ht,
zval *dim, int type TSRMLS_DC)
+static inline zval **zend_fetch_dimension_address_inner(HashTable *ht,
zval *dim, int type, zend_ulong hval, int usehval TSRMLS_DC)
 {
 	zval **retval;
 	char *offset_key;
 	int offset_key_length;
+	int ret;
 
 	switch (dim->type) {
 		case IS_NULL:
@@ -948,7 +949,13 @@
 			offset_key_length = dim->value.str.len;
 			
 fetch_string_dim:
-			if (zend_symtable_find(ht, offset_key, offset_key_length+1, (void
**) &retval) == FAILURE) {
+			if (usehval) {
+				ret = zend_symtable_quick_find(ht, offset_key,
offset_key_length+1, hval, (void **) &retval);
+			}
+			else {
+				ret = zend_symtable_find(ht, offset_key, offset_key_length+1,
(void **) &retval);
+			}
+			if (ret == FAILURE) {
 				switch (type) {
 					case BP_VAR_R:
 						zend_error(E_NOTICE, "Undefined index:  %s", offset_key);
@@ -1023,7 +1030,7 @@
 	return retval;
 }
 
-static void zend_fetch_dimension_address(temp_variable *result, zval
**container_ptr, zval *dim, int dim_is_tmp_var, int type TSRMLS_DC)
+static void zend_fetch_dimension_address(temp_variable *result, zval
**container_ptr, zval *dim, int dim_is_tmp_var, int type, zend_ulong
hval, int usehval TSRMLS_DC)
 {
 	zval *container;
 
@@ -1078,7 +1085,7 @@
 					new_zval->refcount--;
 				}
 			} else {
-				retval = zend_fetch_dimension_address_inner(Z_ARRVAL_P(container),
dim, type TSRMLS_CC);
+				retval = zend_fetch_dimension_address_inner(Z_ARRVAL_P(container),
dim, type, hval, usehval TSRMLS_CC);
 			}
 			if (result) {
 				result->var.ptr_ptr = retval;
diff -r 00438f7eebe4 php-5.2.9RC3/Zend/zend_hash.h
--- a/php-5.2.9RC3/Zend/zend_hash.h	Tue Mar 17 11:27:02 2009 -0700
+++ b/php-5.2.9RC3/Zend/zend_hash.h	Fri Mar 27 10:18:13 2009 -0700
@@ -354,6 +354,12 @@
 	return zend_hash_find(ht, arKey, nKeyLength, pData);
 }
 
+static inline int zend_symtable_quick_find(HashTable *ht, char *arKey,
uint nKeyLength, ulong h, void **pData)
+{
+	HANDLE_NUMERIC(arKey, nKeyLength, zend_hash_index_find(ht, idx,
pData));
+	return zend_hash_quick_find(ht, arKey, nKeyLength, h, pData);
+}
+
 
 static inline int zend_symtable_exists(HashTable *ht, char *arKey,
uint nKeyLength)
 {
diff -r 00438f7eebe4 php-5.2.9RC3/Zend/zend_object_handlers.c
--- a/php-5.2.9RC3/Zend/zend_object_handlers.c	Tue Mar 17 11:27:02 2009
-0700
+++ b/php-5.2.9RC3/Zend/zend_object_handlers.c	Fri Mar 27 10:18:13 2009
-0700
@@ -175,24 +175,11 @@
 	return 0;
 }
 
-ZEND_API struct _zend_property_info
*zend_get_property_info(zend_class_entry *ce, zval *member, int silent
TSRMLS_DC)
+static struct _zend_property_info
*zend_get_property_info_hval(zend_class_entry *ce, zval *member, int
silent, ulong h TSRMLS_DC)
 {
 	zend_property_info *property_info = NULL;
 	zend_property_info *scope_property_info;
 	zend_bool denied_access = 0;
-	ulong h;
-
-	if (Z_STRVAL_P(member)[0] == '\0') {
-		if (!silent) {
-			if (Z_STRLEN_P(member) == 0) {
-				zend_error(E_ERROR, "Cannot access empty property");
-			} else {
-				zend_error(E_ERROR, "Cannot access property started with '\\0'");
-			}
-		}
-		return NULL;
-	}
-	h = zend_get_hash_value(Z_STRVAL_P(member), Z_STRLEN_P(member) + 1);
 	if (zend_hash_quick_find(&ce->properties_info, Z_STRVAL_P(member),
Z_STRLEN_P(member)+1, h, (void **) &property_info)==SUCCESS) {
 		if(property_info->flags & ZEND_ACC_SHADOW) {
 			/* if it's a shadow - go to access it's private */
@@ -244,6 +231,31 @@
 	return property_info;
 }
 
+static int test_valid_string(const zval* member, int silent)
+{
+	if (Z_STRVAL_P(member)[0] == '\0') {
+		if (!silent) {
+			if (Z_STRLEN_P(member) == 0) {
+				zend_error(E_ERROR, "Cannot access empty property");
+			} else {
+				zend_error(E_ERROR, "Cannot access property started with '\\0'");
+			}
+		}
+		return FAILURE;
+	}
+	return SUCCESS;
+}
+
+ZEND_API struct _zend_property_info
*zend_get_property_info(zend_class_entry *ce, zval *member, int silent
TSRMLS_DC)
+{
+	ulong h;
+
+	if (test_valid_string(member, silent) != SUCCESS)
+		return NULL;
+	h = zend_get_hash_value(Z_STRVAL_P(member), Z_STRLEN_P(member) + 1);
+	return zend_get_property_info_hval(ce, member, silent, h TSRMLS_CC);
+}
+
 
 ZEND_API int zend_check_property_access(zend_object *zobj, char
*prop_info_name, int prop_info_name_len TSRMLS_DC)
 {
@@ -293,7 +305,7 @@
 	return zend_hash_quick_add(zobj->guards, property_info->name,
property_info->name_length+1, property_info->h, (void**)&stub,
sizeof(stub), (void**) pguard);
 }
 
-zval *zend_std_read_property(zval *object, zval *member, int type
TSRMLS_DC)
+zval *zend_std_read_property_hval(zval *object, zval *member, ulong
hval, int type TSRMLS_DC)
 {
 	zend_object *zobj;
 	zval *tmp_member = NULL;
@@ -312,14 +324,24 @@
 		zval_copy_ctor(tmp_member);
 		convert_to_string(tmp_member);
 		member = tmp_member;
+		hval = zend_get_hash_value(Z_STRVAL_P(member), Z_STRLEN_P(member));
 	}
+
+	if (test_valid_string(member, silent) != SUCCESS) {
+		if (!silent) {
+			zend_error(E_NOTICE,"Undefined property type for class: %s",
zobj->ce->name);
+		}
+		retval = &EG(uninitialized_zval_ptr);
+		/* Unexpected */
+		return *retval;
+	}
+	property_info = zend_get_property_info_hval(zobj->ce, member,
(zobj->ce->__get != NULL), hval TSRMLS_CC);
 
 #if DEBUG_OBJECT_HANDLERS
 	fprintf(stderr, "Read object #%d property: %s\n",
Z_OBJ_HANDLE_P(object), Z_STRVAL_P(member));
 #endif			
 
 	/* make zend_get_property_info silent if we have getter - we may want
to use it */
-	property_info = zend_get_property_info(zobj->ce, member,
(zobj->ce->__get != NULL) TSRMLS_CC);
 
 	if (!property_info || zend_hash_quick_find(zobj->properties,
property_info->name, property_info->name_length+1, property_info->h,
(void **) &retval) == FAILURE) {
 		zend_guard *guard;
@@ -367,6 +389,15 @@
 		(*retval)->refcount--;
 	}
 	return *retval;
+}
+
+zval *zend_std_read_property(zval *object, zval *member, int type
TSRMLS_DC)
+{
+	ulong hval = 0;
+ 	if (member->type == IS_STRING) {
+		hval = zend_get_hash_value(Z_STRVAL_P(member), Z_STRLEN_P(member));
+	}
+	return zend_std_read_property_hval(object, member, hval, type
TSRMLS_CC);
 }
 
 
diff -r 00438f7eebe4 php-5.2.9RC3/Zend/zend_opcode.c
--- a/php-5.2.9RC3/Zend/zend_opcode.c	Tue Mar 17 11:27:02 2009 -0700
+++ b/php-5.2.9RC3/Zend/zend_opcode.c	Fri Mar 27 10:18:13 2009 -0700
@@ -397,6 +397,32 @@
 			case ZEND_JMPNZ_EX:
 				opline->op2.u.jmp_addr =
&op_array->opcodes[opline->op2.u.opline_num];
 				break;
+			case ZEND_FETCH_OBJ_R:
+			case ZEND_FETCH_OBJ_IS:
+			case ZEND_FETCH_OBJ_FUNC_ARG:
+			case ZEND_FETCH_DIM_R:
+			case ZEND_FETCH_DIM_W:
+			case ZEND_FETCH_DIM_RW:
+			case ZEND_FETCH_DIM_IS:
+			case ZEND_FETCH_DIM_UNSET:
+			case ZEND_ASSIGN_DIM:
+				if ((opline->op2.op_type == IS_CONST) &&
+					(opline->op2.u.constant.type == IS_STRING)
+					) {
+					zval* zop2= &opline->op2.u.constant;
+					opline->hval = zend_get_hash_value(Z_STRVAL_P(zop2),
+												  	   Z_STRLEN_P(zop2) +1);
+				}
+				break;
+			case ZEND_DO_FCALL:
+				if ((opline->op1.op_type == IS_CONST) &&
+					(opline->op1.u.constant.type == IS_STRING)
+					) {
+					zval* zop1= &opline->op1.u.constant;
+					opline->hval = zend_get_hash_value(Z_STRVAL_P(zop1),
+												  	   Z_STRLEN_P(zop1) +1);
+				}
+				break;
 		}
 		ZEND_VM_SET_OPCODE_HANDLER(opline);
 		opline++;
diff -r 00438f7eebe4 php-5.2.9RC3/Zend/zend_vm_def.h
--- a/php-5.2.9RC3/Zend/zend_vm_def.h	Tue Mar 17 11:27:02 2009 -0700
+++ b/php-5.2.9RC3/Zend/zend_vm_def.h	Fri Mar 27 10:18:13 2009 -0700
@@ -418,7 +418,7 @@
 					zend_op *op_data = opline+1;
 					zval *dim = GET_OP2_ZVAL_PTR(BP_VAR_R);
 
-					zend_fetch_dimension_address(&EX_T(op_data->op2.u.var),
GET_OP1_ZVAL_PTR_PTR(BP_VAR_RW), dim, IS_OP2_TMP_FREE(), BP_VAR_RW
TSRMLS_CC);
+					zend_fetch_dimension_address(&EX_T(op_data->op2.u.var),
GET_OP1_ZVAL_PTR_PTR(BP_VAR_RW), dim, IS_OP2_TMP_FREE(), BP_VAR_RW, 0, 0
TSRMLS_CC);
 					value = get_zval_ptr(&op_data->op1, EX(Ts), &free_op_data1,
BP_VAR_R);
 					var_ptr = get_zval_ptr_ptr(&op_data->op2, EX(Ts), &free_op_data2,
BP_VAR_RW);
 					increment_opline = 1;
@@ -1040,13 +1040,17 @@
 	zend_op *opline = EX(opline);
 	zend_free_op free_op1, free_op2;
 	zval *dim = GET_OP2_ZVAL_PTR(BP_VAR_R);
+	int usehval = 0;
+	if (OP2_TYPE == IS_CONST) {
+		usehval = 1;
+	}
 
 	if (opline->extended_value == ZEND_FETCH_ADD_LOCK &&
 	    OP1_TYPE != IS_CV &&
 	    EX_T(opline->op1.u.var).var.ptr_ptr) {
 		PZVAL_LOCK(*EX_T(opline->op1.u.var).var.ptr_ptr);
 	}
-	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
GET_OP1_ZVAL_PTR_PTR(BP_VAR_R), dim, IS_OP2_TMP_FREE(), BP_VAR_R
TSRMLS_CC);
+	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
GET_OP1_ZVAL_PTR_PTR(BP_VAR_R), dim, IS_OP2_TMP_FREE(), BP_VAR_R,
opline->hval, usehval TSRMLS_CC);
 	FREE_OP2();
 	FREE_OP1_VAR_PTR();
 	ZEND_VM_NEXT_OPCODE();
@@ -1058,7 +1062,11 @@
 	zend_free_op free_op1, free_op2;
 	zval *dim = GET_OP2_ZVAL_PTR(BP_VAR_R);
 
-	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
GET_OP1_ZVAL_PTR_PTR(BP_VAR_W), dim, IS_OP2_TMP_FREE(), BP_VAR_W
TSRMLS_CC);
+	int usehval = 0;
+	if (OP2_TYPE == IS_CONST) {
+		usehval = 1;
+	}
+	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
GET_OP1_ZVAL_PTR_PTR(BP_VAR_W), dim, IS_OP2_TMP_FREE(), BP_VAR_W,
opline->hval, usehval TSRMLS_CC);
 	FREE_OP2();
 	if (OP1_TYPE == IS_VAR && OP1_FREE &&
 	    READY_TO_DESTROY(free_op1.var) &&
@@ -1079,7 +1087,11 @@
 	zend_free_op free_op1, free_op2;
 	zval *dim = GET_OP2_ZVAL_PTR(BP_VAR_R);
 
-	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
GET_OP1_ZVAL_PTR_PTR(BP_VAR_RW), dim, IS_OP2_TMP_FREE(), BP_VAR_RW
TSRMLS_CC);
+	int usehval = 0;
+	if (OP2_TYPE == IS_CONST) {
+		usehval = 1;
+	}
+	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
GET_OP1_ZVAL_PTR_PTR(BP_VAR_RW), dim, IS_OP2_TMP_FREE(), BP_VAR_RW,
opline->hval, usehval TSRMLS_CC);
 	FREE_OP2();
 	if (OP1_TYPE == IS_VAR && OP1_FREE &&
 	    READY_TO_DESTROY(free_op1.var) &&
@@ -1099,8 +1111,12 @@
 	zend_op *opline = EX(opline);
 	zend_free_op free_op1, free_op2;
 	zval *dim = GET_OP2_ZVAL_PTR(BP_VAR_R);
+	int usehval = 0;
+	if (OP2_TYPE == IS_CONST) {
+		usehval = 1;
+	}
 
-	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
GET_OP1_ZVAL_PTR_PTR(BP_VAR_IS), dim, IS_OP2_TMP_FREE(), BP_VAR_IS
TSRMLS_CC);
+	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
GET_OP1_ZVAL_PTR_PTR(BP_VAR_IS), dim, IS_OP2_TMP_FREE(), BP_VAR_IS,
opline->hval, usehval TSRMLS_CC);
 	FREE_OP2();
 	FREE_OP1_VAR_PTR();
 	ZEND_VM_NEXT_OPCODE();
@@ -1117,7 +1133,7 @@
 		zend_error_noreturn(E_ERROR, "Cannot use [] for reading");
 	}
 	dim = GET_OP2_ZVAL_PTR(BP_VAR_R);
-	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
GET_OP1_ZVAL_PTR_PTR(type), dim, IS_OP2_TMP_FREE(), type TSRMLS_CC);
+	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
GET_OP1_ZVAL_PTR_PTR(type), dim, IS_OP2_TMP_FREE(), type, 0, 0
TSRMLS_CC);
 	FREE_OP2();
 	if (OP1_TYPE == IS_VAR && type == BP_VAR_W && OP1_FREE &&
 	    READY_TO_DESTROY(free_op1.var) &&
@@ -1138,6 +1154,10 @@
 	zend_free_op free_op1, free_op2;
 	zval **container = GET_OP1_ZVAL_PTR_PTR(BP_VAR_UNSET);
 	zval *dim = GET_OP2_ZVAL_PTR(BP_VAR_R);
+	int usehval = 0;
+	if (OP2_TYPE == IS_CONST) {
+		usehval = 1;
+	}
 
 	/* Not needed in DIM_UNSET
 	if (opline->extended_value == ZEND_FETCH_ADD_LOCK) {
@@ -1149,7 +1169,7 @@
 			SEPARATE_ZVAL_IF_NOT_REF(container);
 		}
 	}
-	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
container, dim, IS_OP2_TMP_FREE(), BP_VAR_UNSET TSRMLS_CC);
+	zend_fetch_dimension_address(RETURN_VALUE_UNUSED(&opline->result)?NULL:&EX_T(opline->result.u.var),
container, dim, IS_OP2_TMP_FREE(), BP_VAR_UNSET, opline->hval, usehval
TSRMLS_CC);
 	FREE_OP2();
 	if (OP1_TYPE == IS_VAR && OP1_FREE &&
 	    READY_TO_DESTROY(free_op1.var) &&
@@ -1176,6 +1196,9 @@
 	ZEND_VM_NEXT_OPCODE();
 }
 
+extern zval *zend_std_read_property(zval *object, zval *member, int
type TSRMLS_DC);
+extern zval *zend_std_read_property_hval(zval *object, zval *member,
ulong hval, int type TSRMLS_DC);
+
 ZEND_VM_HELPER_EX(zend_fetch_property_address_read_helper,
VAR|UNUSED|CV, CONST|TMP|VAR|CV, int type)
 {
 	zend_op *opline = EX(opline);
@@ -1216,7 +1239,14 @@
 		}
 
 		/* here we are sure we are dealing with an object */
-		*retval = Z_OBJ_HT_P(container)->read_property(container, offset,
type TSRMLS_CC);
+		if ((OP2_TYPE == IS_CONST) && Z_OBJ_HT_P(container)->read_property
== zend_std_read_property)
+		{
+			*retval = zend_std_read_property_hval(container, offset,
+												  EX(opline)->hval,
+												  type TSRMLS_CC);
+		} else {
+			*retval = Z_OBJ_HT_P(container)->read_property(container, offset,
type TSRMLS_CC);
+		}
 
 		if (RETURN_VALUE_UNUSED(&opline->result) && ((*retval)->refcount ==
0)) {
 			zval_dtor(*retval);
@@ -1398,7 +1428,7 @@
 		zend_free_op free_op2;
 		zval *dim = GET_OP2_ZVAL_PTR(BP_VAR_R);
 
-		EX_T(opline->result.u.var).var.ptr_ptr =
zend_fetch_dimension_address_inner(Z_ARRVAL_P(container), dim, BP_VAR_R
TSRMLS_CC);
+		EX_T(opline->result.u.var).var.ptr_ptr =
zend_fetch_dimension_address_inner(Z_ARRVAL_P(container), dim, BP_VAR_R,
0, 0 TSRMLS_CC);
 		SELECTIVE_PZVAL_LOCK(*EX_T(opline->result.u.var).var.ptr_ptr,
&opline->result);
 		FREE_OP2();
 	}
@@ -1426,6 +1456,10 @@
 	zend_op *op_data = opline+1;
 	zend_free_op free_op1;
 	zval **object_ptr;
+	int usehval = 0;
+	if (OP2_TYPE == IS_CONST) {
+		usehval = 1;
+	}
 
 	if (OP1_TYPE == IS_CV || EX_T(opline->op1.u.var).var.ptr_ptr) {
 		/* not an array offset */
@@ -1441,7 +1475,7 @@
 		zval *value;
 		zval *dim = GET_OP2_ZVAL_PTR(BP_VAR_R);
 
-		zend_fetch_dimension_address(&EX_T(op_data->op2.u.var), object_ptr,
dim, IS_OP2_TMP_FREE(), BP_VAR_W TSRMLS_CC);
+		zend_fetch_dimension_address(&EX_T(op_data->op2.u.var), object_ptr,
dim, IS_OP2_TMP_FREE(), BP_VAR_W, opline->hval, usehval TSRMLS_CC);
 		FREE_OP2();
 
 		value = get_zval_ptr(&op_data->op1, EX(Ts), &free_op_data1,
BP_VAR_R);
@@ -2098,7 +2132,7 @@
 
 	zend_ptr_stack_3_push(&EG(arg_types_stack), EX(fbc), EX(object),
NULL);
 
-	if (zend_hash_find(EG(function_table), fname->value.str.val,
fname->value.str.len+1, (void **)
&EX(function_state).function)==FAILURE) {
+	if (zend_hash_quick_find(EG(function_table), fname->value.str.val,
fname->value.str.len+1, opline->hval, (void **)
&EX(function_state).function)==FAILURE) {
 		zend_error_noreturn(E_ERROR, "Call to undefined function %s()",
fname->value.str.val);
 	}
 	EX(object) = NULL;


Previous Comments:
------------------------------------------------------------------------

[2009-03-27 22:58:50] basant dot kukreja at gmail dot com

Description:
------------
In specweb ecommerce php benchmark, hash lookup consumes a lot of
time.

For a 30 minutes measurement :
Excl.     Incl.     Name  
User CPU  User CPU       
    sec.      sec.     
 434.304   914.980   zend_fetch_dimension_address
 136.195   427.119   zend_get_property_info

914 seconds (13 % of user time) are spent in
zend_fetch_dimension_address out
of total 6947 seconds.



Reproduce code:
---------------
specweb benchmark.

Expected result:
----------------
Low time spent in zend_fetch_dimension_address and
zend_get_property_info

Actual result:
--------------
Time spend in zend_fetch_dimension_address and zend_get_property_info
is very high.


------------------------------------------------------------------------


-- 
Edit this bug report at http://bugs.php.net/?id=47815&edit=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.