Edit report at http://bugs.php.net/bug.php?id=47815&edit=1
ID: 47815 Updated by: dmi...@php.net Reported by: basant dot kukreja at gmail dot com Summary: [PATCH] Compile time computation of hash value decrease hash lookup time -Status: Assigned +Status: Closed Type: Feature/Change Request Package: *General Issues Operating System: * PHP Version: 5.*, 6 Assigned To: dmitry New Comment: This bug has been fixed in SVN. Snapshots of the sources are packaged every three hours; this change will be in the next snapshot. You can grab the snapshot at http://snaps.php.net/. Thank you for the report, and for helping us make PHP better. Previous Comments: ------------------------------------------------------------------------ [2010-04-20 17:49:00] dmi...@php.net The very similar generalized patches based on the idea from this request are committed into SVN trunk. Sorry, but it's not possible to use such patches in 5.2 and 5.3 because of binary incompatibility. ------------------------------------------------------------------------ [2009-03-30 21:48:55] basant dot kukreja at gmail dot com Regarding adding new field in "opcode" : Ideally I think it would be best if we could have used extended_value for the hash value but some of the relevant opcodes were already using this field. I understand that adding one field in opcode can cause reduction in performance in some scenarios. Other possible solutions which come to my mind is (which is kind of hack): * Encode hash value inside string e.g zval.value.str.val = "abc\0<hash_value> zval.value.str.len = 3 --------------------------------------------- Regarding using zend_std_read_property_hval : Ideally I would like to add a parameter in zend_std_read_property but it would break other object handlers. One possibility is to add a function pointer in _zend_object_handlers e.g typedef zval *(*zend_object_read_property_hash_t)(zval *object, zval *member, hlong hval, int type TSRMLS_DC); struct _zend_object_handlers { ... zend_object_read_property_hash_t read_property_hash; }; And then in opcode handler, we can write : if ((OP2_TYPE == IS_CONST) && Z_OBJ_HT_P(container)->read_property_hash) { *retval = Z_OBJ_HT_P(container)->read_property_hash(container, offset, EX(opline)->hval, type TSRMLS_CC); } else { *retval = Z_OBJ_HT_P(container)->read_property(container, offset, type TSRMLS_CC); } ------------------------------------------------------------------------ [2009-03-30 12:48:04] dmi...@php.net I like the idea, but I don't like some implementation details (addition field in zend_opcode, tricky zend_std_read_property_hash() invocation). However, I know that it's probably not possible to do it better. I would like to make some benchmark tests, but the patch is broken. Could you please put the patch somewhere or send it by email. ------------------------------------------------------------------------ [2009-03-27 23:02:12] basant dot kukreja at gmail dot com Some signifiant percentage of the time is spent in calculating the hash value of string contants. If we compute the hash value of string constants during compilation then lookup time can be improved a lot. With the above submitted patch results are better : Excl. Incl. Name User CPU User CPU sec. sec. 414.450 726.638 zend_fetch_dimension_address 74.922 238.016 zend_get_property_info_hval Note the 150 second (~20 % time) less time spent in zend_fetch_dimension_address and 190 second (45% time) reduction in zend_get_property_info. It showed 1% performance overall. ------------------------------------------------------------------------ [2009-03-27 22:59:55] basant dot kukreja at gmail dot com 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; ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/bug.php?id=47815 -- Edit this bug report at http://bugs.php.net/bug.php?id=47815&edit=1