Everything

php

Runtime-JIT, the whole enchilada

php.internals

Sara Golemon

19 years ago

Dmitry-

You asked for it, you get it :)  Hopefully it'll be self-explanatory,
the one part I hope you don't notice is that I put the auto_global check
back into fetch_simple_variable_ex in order to force direct access to
autoglobals ($foo = $_GET;    foreach($_POST as ...)  etc...) as
non-auto-globals.  Ultimately this made catching the difference between
a whole var access and a dim/obj access much more straight-forward and
computationally cheap.  It comes at a slight cost for those fetches, but
hopefully they're in the minority.

Bench numbers with this patch:
simple             0.461
simplecall         1.985
simpleucall        2.900
simpleudcall       3.488
mandel             2.136
mandel2            3.192
ackermann(7)       3.490
ary(50000)         0.154
ary2(50000)        0.137
ary3(2000)         1.076
fibo(30)           9.766
hash1(50000)       0.423
hash2(500)         0.307
heapsort(20000)    0.799
matrix(20)         0.526
nestedloop(12)     0.866
sieve(30)          0.630
strcat(200000)     0.303
------------------------
Total             32.639

-Sara

Ilia A.

19 years ago

What are the numbers before the patch?

On 24-Jan-07, at 2:58 PM, Sara Golemon wrote:

> Dmitry-
>
>   You asked for it, you get it :)  Hopefully it'll be self-
> explanatory, the one part I hope you don't notice is that I put the
> auto_global check back into fetch_simple_variable_ex in order to
> force direct access to autoglobals ($foo = $_GET;    foreach($_POST
> as ...)  etc...) as non-auto-globals.  Ultimately this made
> catching the difference between a whole var access and a dim/obj
> access much more straight-forward and computationally cheap.  It
> comes at a slight cost for those fetches, but hopefully they're in
> the minority.
>
> Bench numbers with this patch:
> simple             0.461
> simplecall         1.985
> simpleucall        2.900
> simpleudcall       3.488
> mandel             2.136
> mandel2            3.192
> ackermann(7)       3.490
> ary(50000)         0.154
> ary2(50000)        0.137
> ary3(2000)         1.076
> fibo(30)           9.766
> hash1(50000)       0.423
> hash2(500)         0.307
> heapsort(20000)    0.799
> matrix(20)         0.526
> nestedloop(12)     0.866
> sieve(30)          0.630
> strcat(200000)     0.303
> ------------------------
> Total             32.639
>
> -Sara
>
> Index: Zend/zend_compile.c
> ===================================================================
> RCS file: /repository/ZendEngine2/zend_compile.c,v
> retrieving revision 1.736
> diff -u -p -r1.736 zend_compile.c
> --- Zend/zend_compile.c	20 Jan 2007 20:36:55 -0000	1.736
> +++ Zend/zend_compile.c	24 Jan 2007 19:50:52 -0000
> @@ -290,7 +290,8 @@ static int lookup_cv(zend_op_array *op_a
>  	op_array->vars[i].name = name; /* estrndup(name, name_len); */
>  	op_array->vars[i].name_len = name_len;
>  	op_array->vars[i].hash_value = hash_value;
> -	op_array->vars[i].fetch_type = zend_u_is_auto_global(type, name,
> name_len TSRMLS_CC) ? ZEND_FETCH_GLOBAL : ZEND_FETCH_LOCAL;
> +	op_array->vars[i].auto_global = NULL;
> +	op_array->vars[i].fetch_type = zend_u_is_auto_global(type, name,
> name_len, &(op_array->vars[i].auto_global) TSRMLS_CC) ?
> ZEND_FETCH_GLOBAL : ZEND_FETCH_LOCAL;
>  	return i;
>  }
>
> @@ -383,6 +384,7 @@ void fetch_simple_variable_ex(znode *res
>  	     Z_TYPE(varname->u.constant) == IS_UNICODE) &&
>  	    !(Z_UNILEN(varname->u.constant) == (sizeof("this")-1) &&
>  	      ZEND_U_EQUAL(Z_TYPE(varname->u.constant), Z_UNIVAL(varname-
> >u.constant), Z_UNILEN(varname->u.constant), "this", sizeof
> ("this")-1)) &&
> +	    !zend_u_is_auto_global(Z_TYPE(varname->u.constant), Z_UNIVAL
> (varname->u.constant), Z_UNILEN(varname->u.constant), NULL
> TSRMLS_CC) &&
>  	    (CG(active_op_array)->last == 0 ||
>  	     CG(active_op_array)->opcodes[CG(active_op_array)-
> >last-1].opcode != ZEND_BEGIN_SILENCE)) {
>  		result->op_type = IS_CV;
> @@ -410,7 +412,7 @@ void fetch_simple_variable_ex(znode *res
>  	if (varname->op_type == IS_CONST &&
>  	    (Z_TYPE(varname->u.constant) == IS_STRING ||
>  	     Z_TYPE(varname->u.constant) == IS_UNICODE)) {
> -		if (zend_u_is_auto_global(Z_TYPE(varname->u.constant), Z_UNIVAL
> (varname->u.constant), Z_UNILEN(varname->u.constant) TSRMLS_CC)) {
> +		if (zend_u_is_auto_global(Z_TYPE(varname->u.constant), Z_UNIVAL
> (varname->u.constant), Z_UNILEN(varname->u.constant), NULL
> TSRMLS_CC)) {
>  			opline_ptr->op2.u.EA.type = ZEND_FETCH_GLOBAL;
>  		}
>  	}
> @@ -492,6 +494,29 @@ void fetch_array_dim(znode *result, znod
>  	zend_op opline;
>  	zend_llist *fetch_list_ptr;
>
> +	zend_stack_top(&CG(bp_stack), (void **) &fetch_list_ptr);
> +	if (fetch_list_ptr->count == 1) {
> +		zend_llist_element *le = fetch_list_ptr->head;
> +		zend_op *parentop = (zend_op*)le->data;
> +
> +		if (parentop && parentop->opcode == ZEND_FETCH_W &&
> +			parent->op_type == IS_VAR && parentop->result.op_type == IS_VAR
> && parent->u.var == parentop->result.u.var &&
> +			parentop->op1.op_type == IS_CONST &&
> +			(Z_TYPE(parentop->op1.u.constant) == IS_STRING || Z_TYPE
> (parentop->op1.u.constant) == IS_UNICODE) &&
> + 			!(Z_UNILEN(parentop->op1.u.constant) == (sizeof("this")-1) &&
> ZEND_U_EQUAL(Z_TYPE(parentop->op1.u.constant), Z_UNIVAL(parentop-
> >op1.u.constant), Z_UNILEN(parentop->op1.u.constant), "this", sizeof
> ("this")-1)) ) {
> +			/* Recompile CV and rewrite previous op to direct FETCH_DIM */
> +			zval tmp = parentop->op1.u.constant;
> +			parentop->opcode = ZEND_FETCH_DIM_W;
> +			parentop->op1.op_type = IS_CV;
> +			parentop->op1.u.var = lookup_cv(CG(active_op_array), Z_TYPE
> (tmp), Z_UNIVAL(tmp), Z_UNILEN(tmp) TSRMLS_CC);
> +			parentop->op1.u.EA.type = 0;
> +			parentop->op2 = *dim;
> +			parentop->extended_value = ZEND_FETCH_STANDARD;
> +			*result = parentop->result;
> +			return;
> +		}
> +	}
> +
>  	init_op(&opline TSRMLS_CC);
>  	opline.opcode = ZEND_FETCH_DIM_W;	/* the backpatching routine
> assumes W */
>  	opline.result.op_type = IS_VAR;
> @@ -502,7 +527,6 @@ void fetch_array_dim(znode *result, znod
>  	opline.extended_value = ZEND_FETCH_STANDARD;
>  	*result = opline.result;
>
> -	zend_stack_top(&CG(bp_stack), (void **) &fetch_list_ptr);
>  	zend_llist_add_element(fetch_list_ptr, &opline);
>  }
>
> @@ -3261,7 +3285,6 @@ void zend_do_fetch_property(znode *resul
>  	zend_op *opline_ptr=NULL;
>
>  	zend_stack_top(&CG(bp_stack), (void **) &fetch_list_ptr);
> -
>  	if (fetch_list_ptr->count == 1) {
>  		zend_llist_element *le;
>
> @@ -3295,6 +3318,19 @@ void zend_do_fetch_property(znode *resul
>  			}
>  			*result = opline_ptr->result;
>  			return;
> +		} else if (opline_ptr && opline_ptr->opcode == ZEND_FETCH_W &&
> +			object->op_type == IS_VAR && opline_ptr->result.op_type ==
> IS_VAR && object->u.var == opline_ptr->result.u.var &&
> +			opline_ptr->op1.op_type == IS_CONST &&
> +			(Z_TYPE(opline_ptr->op1.u.constant) == IS_STRING || Z_TYPE
> (opline_ptr->op1.u.constant) == IS_UNICODE) ) {
> +			/* Recompile CV and rewrite previous op to direct FETCH_OBJ */
> +			zval tmp = opline_ptr->op1.u.constant;
> +			opline_ptr->opcode = ZEND_FETCH_OBJ_W;
> +			opline_ptr->op1.op_type = IS_CV;
> +			opline_ptr->op1.u.var = lookup_cv(CG(active_op_array), Z_TYPE
> (tmp), Z_UNIVAL(tmp), Z_UNILEN(tmp) TSRMLS_CC);
> +			opline_ptr->op1.u.EA.type = 0;
> +			opline_ptr->op2 = *property;
> +			*result = opline_ptr->result;
> +			return;
>  		}
>  	}
>
> @@ -4312,13 +4348,16 @@ void zend_auto_global_dtor(zend_auto_glo
>  }
>
>
> -zend_bool zend_u_is_auto_global(zend_uchar type, zstr name, uint
> name_len TSRMLS_DC)
> +zend_bool zend_u_is_auto_global(zend_uchar type, zstr name, uint
> name_len, zend_auto_global **pauto TSRMLS_DC)
>  {
>  	zend_auto_global *auto_global;
>
>  	if (zend_u_hash_find(CG(auto_globals), type, name, name_len+1,
> (void **) &auto_global)==SUCCESS) {
>  		if (auto_global->armed) {
> -			auto_global->armed = auto_global->auto_global_callback
> (auto_global->name, auto_global->name_len TSRMLS_CC);
> +			auto_global->armed = auto_global->auto_global_callback
> (auto_global, ZEND_CT, NULL, 0, NULL TSRMLS_CC);
> +		}
> +		if (pauto) {
> +			*pauto = auto_global;
>  		}
>  		return 1;
>  	}
> @@ -4327,21 +4366,26 @@ zend_bool zend_u_is_auto_global(zend_uch
>
>  zend_bool zend_is_auto_global(char *name, uint name_len TSRMLS_DC)
>  {
> -	return zend_u_is_auto_global(IS_STRING, ZSTR(name), name_len
> TSRMLS_CC);
> +	return zend_u_is_auto_global(IS_STRING, ZSTR(name), name_len,
> NULL TSRMLS_CC);
>  }
>
>
> -int zend_register_auto_global(char *name, uint name_len,
> zend_auto_global_callback auto_global_callback TSRMLS_DC)
> +int zend_register_auto_global_ex(char *name, uint name_len,
> zend_auto_global_callback auto_global_callback, zend_auto_global
> **pauto TSRMLS_DC)
>  {
>  	zend_auto_global auto_global;
>
>  	auto_global.name = zend_strndup(name, name_len);
>  	auto_global.name_len = name_len;
>  	auto_global.auto_global_callback = auto_global_callback;
> +	auto_global.armed = auto_global_callback ? 1 : 0;
>
> -	return zend_hash_add(CG(auto_globals), name, name_len+1,
> &auto_global, sizeof(zend_auto_global), NULL);
> +	return zend_hash_add(CG(auto_globals), name, name_len+1,
> &auto_global, sizeof(zend_auto_global), (void**)pauto);
>  }
>
> +int zend_register_auto_global(char *name, uint name_len,
> zend_auto_global_callback auto_global_callback TSRMLS_DC)
> +{
> +	return zend_register_auto_global_ex(name, name_len,
> auto_global_callback, NULL TSRMLS_CC);
> +}
>
>  int zendlex(znode *zendlval TSRMLS_DC)
>  {
> Index: Zend/zend_compile.h
> ===================================================================
> RCS file: /repository/ZendEngine2/zend_compile.h,v
> retrieving revision 1.353
> diff -u -p -r1.353 zend_compile.h
> --- Zend/zend_compile.h	20 Jan 2007 20:36:55 -0000	1.353
> +++ Zend/zend_compile.h	24 Jan 2007 19:50:52 -0000
> @@ -51,6 +51,7 @@
>
>  typedef struct _zend_op_array zend_op_array;
>  typedef struct _zend_op zend_op;
> +typedef struct _zend_auto_global zend_auto_global;
>
>  typedef struct _znode {
>  	int op_type;
> @@ -175,6 +176,7 @@ typedef struct _zend_compiled_variable {
>  	int name_len;
>  	ulong hash_value;
>  	zend_uint fetch_type;
> +	zend_auto_global *auto_global;
>  } zend_compiled_variable;
>
>  struct _zend_op_array {
> @@ -575,18 +577,19 @@ ZEND_API char *zend_make_compiled_string
>  ZEND_API void zend_initialize_class_data(zend_class_entry *ce,
> zend_bool nullify_handlers TSRMLS_DC);
>  int zend_get_class_fetch_type(zend_uchar type, zstr class_name,
> uint class_name_len);
>
> -typedef zend_bool (*zend_auto_global_callback)(char *name, uint
> name_len TSRMLS_DC);
> -typedef struct _zend_auto_global {
> +typedef zend_bool (*zend_auto_global_callback)(zend_auto_global
> *auto_global, int stage, zval *ag_val, int fetch_op, zval *member
> TSRMLS_DC);
> +struct _zend_auto_global {
>  	char *name;
>  	uint name_len;
>  	zend_auto_global_callback auto_global_callback;
>  	zend_bool armed;
> -} zend_auto_global;
> +};
>
>  void zend_auto_global_dtor(zend_auto_global *auto_global);
> +ZEND_API int zend_register_auto_global_ex(char *name, uint
> name_len, zend_auto_global_callback auto_global_callback,
> zend_auto_global **pauto TSRMLS_DC);
>  ZEND_API int zend_register_auto_global(char *name, uint name_len,
> zend_auto_global_callback auto_global_callback TSRMLS_DC);
>  ZEND_API zend_bool zend_is_auto_global(char *name, uint name_len
> TSRMLS_DC);
> -ZEND_API zend_bool zend_u_is_auto_global(zend_uchar type, zstr
> name, uint name_len TSRMLS_DC);
> +ZEND_API zend_bool zend_u_is_auto_global(zend_uchar type, zstr
> name, uint name_len, zend_auto_global **pauto TSRMLS_DC);
>  ZEND_API int zend_auto_global_disable_jit(char *varname, zend_uint
> varname_length TSRMLS_DC);
>
>  int zendlex(znode *zendlval TSRMLS_DC);
> Index: Zend/zend_execute.c
> ===================================================================
> RCS file: /repository/ZendEngine2/zend_execute.c,v
> retrieving revision 1.758
> diff -u -p -r1.758 zend_execute.c
> --- Zend/zend_execute.c	20 Jan 2007 20:36:55 -0000	1.758
> +++ Zend/zend_execute.c	24 Jan 2007 19:50:52 -0000
> @@ -903,8 +903,17 @@ static inline HashTable *zend_get_target
>  			break;
>  		case ZEND_FETCH_GLOBAL:
>  		case ZEND_FETCH_GLOBAL_LOCK:
> +		{
> +			zend_auto_global *auto_global;
> +
> +			if ((Z_TYPE_P(variable) == IS_STRING || Z_TYPE_P(variable) ==
> IS_UNICODE) &&
> +				SUCCESS == zend_u_hash_find(CG(auto_globals), Z_TYPE_P
> (variable), Z_UNIVAL_P(variable), Z_UNILEN_P(variable) + 1, (void
> **) &auto_global) &&
> +				auto_global->armed && auto_global->auto_global_callback) {
> +				auto_global->armed = auto_global->auto_global_callback
> (auto_global, ZEND_RT, NULL, ZEND_FETCH_R + (3 * type), NULL
> TSRMLS_CC);
> +			}
>  			return &EG(symbol_table);
>  			break;
> +		}
>  		case ZEND_FETCH_STATIC:
>  			if (!EG(active_op_array)->static_variables) {
>  				ALLOC_HASHTABLE(EG(active_op_array)->static_variables);
> @@ -1030,7 +1039,7 @@ fetch_string_dim:
>  	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(znode *container_node,
> temp_variable *result, zval **container_ptr, zval *dim, int
> dim_is_tmp_var, int type TSRMLS_DC)
>  {
>  	zval *container;
>
> @@ -1039,6 +1048,13 @@ static void zend_fetch_dimension_address
>  	}
>
>  	container = *container_ptr;
> +	if (container_node->op_type == IS_CV) {
> +		zend_auto_global *auto_global = CV_DEF_OF(container_node-
> >u.var).auto_global;
> +
> +		if (auto_global && auto_global->armed && auto_global-
> >auto_global_callback) {
> +			auto_global->armed = auto_global->auto_global_callback
> (auto_global, ZEND_RT, container, ZEND_FETCH_DIM_R + (3 * type),
> dim TSRMLS_CC);
> +		}
> +	}
>
>  	if (container == EG(error_zval_ptr)) {
>  		if (result) {
> @@ -1232,11 +1248,19 @@ static void zend_fetch_dimension_address
>  	}
>  }
>
> -static void zend_fetch_property_address(temp_variable *result,
> zval **container_ptr, zval *prop_ptr, int type TSRMLS_DC)
> +static void zend_fetch_property_address(znode *container_node,
> temp_variable *result, zval **container_ptr, zval *prop_ptr, int
> type TSRMLS_DC)
>  {
>  	zval *container;
>
>  	container = *container_ptr;
> +	if (container_node->op_type == IS_CV) {
> +		zend_auto_global *auto_global = CV_DEF_OF(container_node-
> >u.var).auto_global;
> +
> +		if (auto_global && auto_global->armed && auto_global-
> >auto_global_callback) {
> +			auto_global->armed = auto_global->auto_global_callback
> (auto_global, ZEND_RT, container, ZEND_FETCH_OBJ_R + (3 * type),
> prop_ptr TSRMLS_CC);
> +		}
> +	}
> +
>  	if (container == EG(error_zval_ptr)) {
>  		if (result) {
>  			result->var.ptr_ptr = &EG(error_zval_ptr);
> Index: Zend/zend_vm_def.h
> ===================================================================
> RCS file: /repository/ZendEngine2/zend_vm_def.h,v
> retrieving revision 1.155
> diff -u -p -r1.155 zend_vm_def.h
> --- Zend/zend_vm_def.h	11 Jan 2007 22:35:36 -0000	1.155
> +++ Zend/zend_vm_def.h	24 Jan 2007 19:50:52 -0000
> @@ -418,7 +418,7 @@ ZEND_VM_HELPER_EX(zend_binary_assign_op_
>  					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(&opline->op1, &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);
>  					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;
> @@ -1066,7 +1066,7 @@ ZEND_VM_HANDLER(81, ZEND_FETCH_DIM_R, VA
>  	    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(&opline->op1, 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);
>  	FREE_OP2();
>  	FREE_OP1_VAR_PTR();
>  	ZEND_VM_NEXT_OPCODE();
> @@ -1078,7 +1078,7 @@ ZEND_VM_HANDLER(84, ZEND_FETCH_DIM_W, VA
>  	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);
> +	zend_fetch_dimension_address(&opline->op1, 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);
>  	FREE_OP2();
>  	if (OP1_TYPE == IS_VAR && OP1_FREE &&
>  	    READY_TO_DESTROY(free_op1.var) &&
> @@ -1095,7 +1095,7 @@ ZEND_VM_HANDLER(87, ZEND_FETCH_DIM_RW, V
>  	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);
> +	zend_fetch_dimension_address(&opline->op1, 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);
>  	FREE_OP2();
>  	if (OP1_TYPE == IS_VAR && OP1_FREE &&
>  	    READY_TO_DESTROY(free_op1.var) &&
> @@ -1112,7 +1112,7 @@ ZEND_VM_HANDLER(90, ZEND_FETCH_DIM_IS, V
>  	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_IS),
> dim, IS_OP2_TMP_FREE(), BP_VAR_IS TSRMLS_CC);
> +	zend_fetch_dimension_address(&opline->op1, 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);
>  	FREE_OP2();
>  	FREE_OP1_VAR_PTR();
>  	ZEND_VM_NEXT_OPCODE();
> @@ -1129,7 +1129,7 @@ ZEND_VM_HANDLER(93, ZEND_FETCH_DIM_FUNC_
>  		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(&opline->op1, 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);
>  	FREE_OP2();
>  	if (OP1_TYPE == IS_VAR && type == BP_VAR_W && OP1_FREE &&
>  	    READY_TO_DESTROY(free_op1.var) &&
> @@ -1157,7 +1157,7 @@ ZEND_VM_HANDLER(96, ZEND_FETCH_DIM_UNSET
>  			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(&opline->op1, RETURN_VALUE_UNUSED
> (&opline->result)?NULL:&EX_T(opline->result.u.var), container, dim,
> IS_OP2_TMP_FREE(), BP_VAR_UNSET TSRMLS_CC);
>  	FREE_OP2();
>  	if (OP1_TYPE == IS_VAR && OP1_FREE &&
>  	    READY_TO_DESTROY(free_op1.var) &&
> @@ -1218,6 +1218,14 @@ ZEND_VM_HELPER_EX(zend_fetch_property_ad
>  			MAKE_REAL_ZVAL_PTR(offset);
>  		}
>
> +		if (opline->op1.op_type == IS_CV) {
> +			zend_auto_global *auto_global = CV_DEF_OF(opline-
> >op1.u.var).auto_global;
> +
> +			if (auto_global && auto_global->armed && auto_global-
> >auto_global_callback) {
> +				auto_global->armed = auto_global->auto_global_callback
> (auto_global, ZEND_RT, container, ZEND_FETCH_OBJ_R + (3 * type),
> offset TSRMLS_CC);
> +			}
> +		}
> +
>  		/* here we are sure we are dealing with an object */
>  		*retval = Z_OBJ_HT_P(container)->read_property(container,
> offset, type TSRMLS_CC);
>
> @@ -1259,7 +1267,7 @@ ZEND_VM_HANDLER(85, ZEND_FETCH_OBJ_W, VA
>  	if (IS_OP2_TMP_FREE()) {
>  		MAKE_REAL_ZVAL_PTR(property);
>  	}
> -	zend_fetch_property_address(RETURN_VALUE_UNUSED(&opline->result)?
> NULL:&EX_T(opline->result.u.var), GET_OP1_OBJ_ZVAL_PTR_PTR
> (BP_VAR_W), property, BP_VAR_W TSRMLS_CC);
> +	zend_fetch_property_address(&opline->op1, RETURN_VALUE_UNUSED
> (&opline->result)?NULL:&EX_T(opline->result.u.var),
> GET_OP1_OBJ_ZVAL_PTR_PTR(BP_VAR_W), property, BP_VAR_W TSRMLS_CC);
>  	if (IS_OP2_TMP_FREE()) {
>  		zval_ptr_dtor(&property);
>  	} else {
> @@ -1283,7 +1291,7 @@ ZEND_VM_HANDLER(88, ZEND_FETCH_OBJ_RW, V
>  	if (IS_OP2_TMP_FREE()) {
>  		MAKE_REAL_ZVAL_PTR(property);
>  	}
> -	zend_fetch_property_address(RETURN_VALUE_UNUSED(&opline->result)?
> NULL:&EX_T(opline->result.u.var), GET_OP1_OBJ_ZVAL_PTR_PTR
> (BP_VAR_RW), property, BP_VAR_RW TSRMLS_CC);
> +	zend_fetch_property_address(&opline->op1, RETURN_VALUE_UNUSED
> (&opline->result)?NULL:&EX_T(opline->result.u.var),
> GET_OP1_OBJ_ZVAL_PTR_PTR(BP_VAR_RW), property, BP_VAR_RW TSRMLS_CC);
>  	if (IS_OP2_TMP_FREE()) {
>  		zval_ptr_dtor(&property);
>  	} else {
> @@ -1315,7 +1323,7 @@ ZEND_VM_HANDLER(94, ZEND_FETCH_OBJ_FUNC_
>  		if (IS_OP2_TMP_FREE()) {
>  			MAKE_REAL_ZVAL_PTR(property);
>  		}
> -		zend_fetch_property_address(RETURN_VALUE_UNUSED(&opline->result)?
> NULL:&EX_T(opline->result.u.var), GET_OP1_OBJ_ZVAL_PTR_PTR
> (BP_VAR_W), property, BP_VAR_W TSRMLS_CC);
> +		zend_fetch_property_address(&opline->op1, RETURN_VALUE_UNUSED
> (&opline->result)?NULL:&EX_T(opline->result.u.var),
> GET_OP1_OBJ_ZVAL_PTR_PTR(BP_VAR_W), property, BP_VAR_W TSRMLS_CC);
>  		if (IS_OP2_TMP_FREE()) {
>  			zval_ptr_dtor(&property);
>  		} else {
> @@ -1348,7 +1356,7 @@ ZEND_VM_HANDLER(97, ZEND_FETCH_OBJ_UNSET
>  	if (IS_OP2_TMP_FREE()) {
>  		MAKE_REAL_ZVAL_PTR(property);
>  	}
> -	zend_fetch_property_address(RETURN_VALUE_UNUSED(&opline->result)?
> NULL:&EX_T(opline->result.u.var), container, property, BP_VAR_R
> TSRMLS_CC);
> +	zend_fetch_property_address(&opline->op1, RETURN_VALUE_UNUSED
> (&opline->result)?NULL:&EX_T(opline->result.u.var), container,
> property, BP_VAR_R TSRMLS_CC);
>  	if (IS_OP2_TMP_FREE()) {
>  		zval_ptr_dtor(&property);
>  	} else {
> @@ -1428,7 +1436,7 @@ ZEND_VM_HANDLER(147, ZEND_ASSIGN_DIM, VA
>  		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(&opline->op1, &EX_T(op_data-
> >op2.u.var), object_ptr, dim, IS_OP2_TMP_FREE(), BP_VAR_W TSRMLS_CC);
>  		FREE_OP2();
>
>  		value = get_zval_ptr(&op_data->op1, EX(Ts), &free_op_data1,
> BP_VAR_R);
> Index: main/php_variables.c
> ===================================================================
> RCS file: /repository/php-src/main/php_variables.c,v
> retrieving revision 1.136
> diff -u -p -r1.136 php_variables.c
> --- main/php_variables.c	1 Jan 2007 09:29:35 -0000	1.136
> +++ main/php_variables.c	24 Jan 2007 19:50:52 -0000
> @@ -628,9 +628,9 @@ void _php_import_environment_variables(z
>  	}
>  }
>
> -zend_bool php_std_auto_global_callback(char *name, uint name_len
> TSRMLS_DC)
> +zend_bool php_std_auto_global_callback(zend_auto_global
> *auto_global, int stage, zval *ag_val, int fetch_op, zval *member
> TSRMLS_DC)
>  {
> -	zend_printf("%s\n", name);
> +	zend_printf("%s\n", auto_global->name);
>  	return 0; /* don't rearm */
>  }
>
> @@ -802,9 +802,13 @@ static void php_autoglobal_merge(HashTab
>  }
>  /* }}} */
>
> -static zend_bool php_auto_globals_create_server(char *name, uint
> name_len TSRMLS_DC);
> -static zend_bool php_auto_globals_create_env(char *name, uint
> name_len TSRMLS_DC);
> -static zend_bool php_auto_globals_create_request(char *name, uint
> name_len TSRMLS_DC);
> +static zend_auto_global *php_server_auto_global = NULL;
> +static zend_auto_global *php_env_auto_global = NULL;
> +static zend_auto_global *php_request_auto_global = NULL;
> +
> +static zend_bool php_auto_globals_create_server(zend_auto_global
> *auto_global, int stage, zval *ag_val, int fetch_op, zval *member
> TSRMLS_DC);
> +static zend_bool php_auto_globals_create_env(zend_auto_global
> *auto_global, int stage, zval *ag_val, int fetch_op, zval *member
> TSRMLS_DC);
> +static zend_bool php_auto_globals_create_request(zend_auto_global
> *auto_global, int stage, zval *ag_val, int fetch_op, zval *member
> TSRMLS_DC);
>
>  /* {{{ php_hash_environment
>   */
> @@ -861,16 +865,14 @@ int php_hash_environment(TSRMLS_D)
>  			case 'e':
>  			case 'E':
>  				if (!jit_initialization && !_gpc_flags[3]) {
> -					zend_auto_global_disable_jit("_ENV", sizeof("_ENV")-1
> TSRMLS_CC);
> -					php_auto_globals_create_env("_ENV", sizeof("_ENV")-1 TSRMLS_CC);
> +					php_env_auto_global->armed = php_auto_globals_create_env
> (php_env_auto_global, ZEND_RT, NULL, ZEND_FETCH_R, NULL TSRMLS_CC);
>  					_gpc_flags[3] = 1;
>  				}
>  				break;
>  			case 's':
>  			case 'S':
>  				if (!jit_initialization && !_gpc_flags[4]) {
> -					zend_auto_global_disable_jit("_SERVER", sizeof("_SERVER")-1
> TSRMLS_CC);
> -					php_register_server_variables(TSRMLS_C);
> +					php_server_auto_global->armed = php_auto_globals_create_server
> (php_server_auto_global, ZEND_RT, NULL, ZEND_FETCH_R, NULL TSRMLS_CC);
>  					_gpc_flags[4] = 1;
>  				}
>  				break;
> @@ -904,15 +906,14 @@ int php_hash_environment(TSRMLS_D)
>
>  	/* Create _REQUEST */
>  	if (!jit_initialization) {
> -		zend_auto_global_disable_jit("_REQUEST", sizeof("_REQUEST")-1
> TSRMLS_CC);
> -		php_auto_globals_create_request("_REQUEST", sizeof("_REQUEST")-1
> TSRMLS_CC);
> +		php_request_auto_global->armed = php_auto_globals_create_request
> (php_request_auto_global, ZEND_RT, NULL, ZEND_FETCH_R, NULL
> TSRMLS_CC);
>  	}
>
>  	return SUCCESS;
>  }
>  /* }}} */
>
> -static zend_bool php_auto_globals_create_server(char *name, uint
> name_len TSRMLS_DC)
> +static zend_bool php_auto_globals_create_server(zend_auto_global
> *auto_global, int stage, zval *ag_val, int fetch_op, zval *member
> TSRMLS_DC)
>  {
>  	if (PG(variables_order) && (strchr(PG(variables_order),'S') ||
> strchr(PG(variables_order),'s'))) {
>  		php_register_server_variables(TSRMLS_C);
> @@ -944,13 +945,13 @@ static zend_bool php_auto_globals_create
>  		PG(http_globals)[TRACK_VARS_SERVER] = server_vars;
>  	}
>
> -	zend_ascii_hash_update(&EG(symbol_table), name, name_len + 1, &PG
> (http_globals)[TRACK_VARS_SERVER], sizeof(zval *), NULL);
> +	zend_ascii_hash_update(&EG(symbol_table), auto_global->name,
> auto_global->name_len + 1, &PG(http_globals)[TRACK_VARS_SERVER],
> sizeof(zval *), NULL);
>  	PG(http_globals)[TRACK_VARS_SERVER]->refcount++;
>
>  	return 0; /* don't rearm */
>  }
>
> -static zend_bool php_auto_globals_create_env(char *name, uint
> name_len TSRMLS_DC)
> +static zend_bool php_auto_globals_create_env(zend_auto_global
> *auto_global, int stage, zval *ag_val, int fetch_op, zval *member
> TSRMLS_DC)
>  {
>  	zval *env_vars = NULL;
>  	ALLOC_ZVAL(env_vars);
> @@ -965,13 +966,13 @@ static zend_bool php_auto_globals_create
>  		php_import_environment_variables(PG(http_globals)
> [TRACK_VARS_ENV] TSRMLS_CC);
>  	}
>
> -	zend_ascii_hash_update(&EG(symbol_table), name, name_len + 1, &PG
> (http_globals)[TRACK_VARS_ENV], sizeof(zval *), NULL);
> +	zend_ascii_hash_update(&EG(symbol_table), auto_global->name,
> auto_global->name_len + 1, &PG(http_globals)[TRACK_VARS_ENV], sizeof
> (zval *), NULL);
>  	PG(http_globals)[TRACK_VARS_ENV]->refcount++;
>
>  	return 0; /* don't rearm */
>  }
>
> -static zend_bool php_auto_globals_create_request(char *name, uint
> name_len TSRMLS_DC)
> +static zend_bool php_auto_globals_create_request(zend_auto_global
> *auto_global, int stage, zval *ag_val, int fetch_op, zval *member
> TSRMLS_DC)
>  {
>  	zval *form_variables;
>  	unsigned char _gpc_flags[3] = {0, 0, 0};
> @@ -1007,7 +1008,7 @@ static zend_bool php_auto_globals_create
>  		}
>  	}
>
> -	zend_ascii_hash_update(&EG(symbol_table), "_REQUEST", sizeof
> ("_REQUEST"), &form_variables, sizeof(zval *), NULL);
> +	zend_ascii_hash_update(&EG(symbol_table), auto_global->name,
> auto_global->name_len + 1, &form_variables, sizeof(zval *), NULL);
>  	return 0;
>  }
>
> @@ -1016,9 +1017,9 @@ void php_startup_auto_globals(TSRMLS_D)
>  	zend_register_auto_global("_GET", sizeof("_GET")-1, NULL TSRMLS_CC);
>  	zend_register_auto_global("_POST", sizeof("_POST")-1, NULL
> TSRMLS_CC);
>  	zend_register_auto_global("_COOKIE", sizeof("_COOKIE")-1, NULL
> TSRMLS_CC);
> -	zend_register_auto_global("_SERVER", sizeof("_SERVER")-1,
> php_auto_globals_create_server TSRMLS_CC);
> -	zend_register_auto_global("_ENV", sizeof("_ENV")-1,
> php_auto_globals_create_env TSRMLS_CC);
> -	zend_register_auto_global("_REQUEST", sizeof("_REQUEST")-1,
> php_auto_globals_create_request TSRMLS_CC);
> +	zend_register_auto_global_ex("_SERVER", sizeof("_SERVER")-1,
> php_auto_globals_create_server, &php_server_auto_global TSRMLS_CC);
> +	zend_register_auto_global_ex("_ENV", sizeof("_ENV")-1,
> php_auto_globals_create_env, &php_env_auto_global TSRMLS_CC);
> +	zend_register_auto_global_ex("_REQUEST", sizeof("_REQUEST")-1,
> php_auto_globals_create_request, &php_request_auto_global TSRMLS_CC);
>  	zend_register_auto_global("_FILES", sizeof("_FILES")-1, NULL
> TSRMLS_CC);
>  }
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php

Ilia Alshanetsky

Dmitry Stogov

19 years ago

Hi Sara,

I fixed one error in your patch, so now it passes all tests.

BTW I am against this patch.

1) It adds sloppily built functionality into the core and slowdowns EACH
dimension or property fetch both at run-time and compilation-time. I think
it is a big cost for $_GET/$_POST encoding-conversion, but it is only my
opinion.

2) It adds pointer to zend_auto_global into zend_compiled_variable. This
makes troubles for opcode caches - zend_compiled_variable(s) may lay in
shared memory, so they cannot point to process specific data. I think this
issue can be fixed using indirect pointer (index).

I would prefer rollback "auto-globals CV" patch and commit (or not) the
whole patch after conclusion.

Why did you reject auto-globals as overloaded arrays?

Thanks. Dmitry.

Pierre Joye

19 years ago

Hi Dmitry,

On 1/26/07, Dmitry Stogov <dmitry@zend.com> wrote:

> Hi Sara,
>
> I fixed one error in your patch, so now it passes all tests.
>
> BTW I am against this patch.
>
> 1) It adds sloppily built functionality into the core and slowdowns EACH
> dimension or property fetch both at run-time and compilation-time. I think
> it is a big cost for $_GET/$_POST encoding-conversion, but it is only my
> opinion.
>
> 2) It adds pointer to zend_auto_global into zend_compiled_variable. This
> makes troubles for opcode caches - zend_compiled_variable(s) may lay in
> shared memory, so they cannot point to process specific data. I think this
> issue can be fixed using indirect pointer (index).
>
> I would prefer rollback "auto-globals CV" patch and commit (or not) the
> whole patch after conclusion.
>
> Why did you reject auto-globals as overloaded arrays?

As I already told you in my initial proposal (which is simpler than
this solution), they do not work in all cases (check the various bugs
about them) and they will bring a huge breakage as is_array($objarray)
returns false. The later is certainly fixable (with slowness too), I'm
not sure about the other issues.

--Pierre

Sara Golemon

19 years ago

> I fixed one error in your patch, so now it passes all tests.
>

Which error?  I notice some changes related to minimizing performance
impact, but I'm not seeing what I did wrong.  Just curious for my own
benefit on that one.

> BTW I am against this patch.
>

Fair enough.

> 1) It adds sloppily built functionality into the core

Well, you sure know how to sweet-talk a gal.

and slowdowns EACH

> dimension or property fetch both at run-time and compilation-time.  I think
> it is a big cost for $_GET/$_POST encoding-conversion, but it is only my
> opinion.
>

For disarmed callbacks, it amounts to a vector lookup and a couple
integer compares, I'd hardly call that a "big cost".  As to armed
callbacks, well, if they're still armed then they still have some work
to do.  Refer to prior posts in the thread as to why this work would
(and should) be deferred.

> 2) It adds pointer to zend_auto_global into zend_compiled_variable. This
> makes troubles for opcode caches - zend_compiled_variable(s) may lay in
> shared memory, so they cannot point to process specific data. I think this
> issue can be fixed using indirect pointer (index).
>

Excellent point.  That will need to be addressed....

> Why did you reject auto-globals as overloaded arrays?
>

See earlier posts in this thread, such as the one I sent you less than a
week ago and to which you replied indicating you understood it.

If you thinking breaking BC is preferable to this "sloppy" approach,
then we can reexamine that.  If you have a better, third approach, which
solves all of the issues already outlined, then I'd love to hear it.

-Sara

Sara Golemon

19 years ago

> I fixed one error in your patch, so now it passes all tests.
>

Which error?  I notice some changes related to minimizing performance
impact, but I'm not seeing what I did wrong.  Just curious for my own
benefit on that one.

> BTW I am against this patch.
>

Fair enough.

> 1) It adds sloppily built functionality into the core

Well, you sure know how to sweet-talk a gal.

and slowdowns EACH

> dimension or property fetch both at run-time and compilation-time.  I think
> it is a big cost for $_GET/$_POST encoding-conversion, but it is only my
> opinion.
>

For disarmed callbacks, it amounts to a vector lookup and a couple
integer compares, I'd hardly call that a "big cost".  As to armed
callbacks, well, if they're still armed then they still have some work
to do.  Refer to prior posts in the thread as to why this work would
(and should) be deferred.

> 2) It adds pointer to zend_auto_global into zend_compiled_variable. This
> makes troubles for opcode caches - zend_compiled_variable(s) may lay in
> shared memory, so they cannot point to process specific data. I think this
> issue can be fixed using indirect pointer (index).
>

Excellent point.  That will need to be addressed....

> Why did you reject auto-globals as overloaded arrays?
>

See earlier posts in this thread, such as the one I sent you less than a
week ago and to which you replied indicating you understood it.

If you thinking breaking BC is preferable to this "sloppy" approach,
then we can reexamine that.  If you have a better, third approach, which
solves all of the issues already outlined, then I'd love to hear it.

-Sara

Dmitry Stogov

19 years ago

Hi Sara,

> -----Original Message-----
> From: Sara Golemon [mailto:pollita@php.net]
> Sent: Friday, January 26, 2007 8:11 PM
> To: Dmitry Stogov
> Cc: internals@lists.php.net; 'Andi Gutmans'; 'Andrei
> Zmievski'; Zeev Suraski; Stanislav Malyshev
> Subject: Re: [PHP-DEV] Runtime-JIT, the whole enchilada
>
>
> > I fixed one error in your patch, so now it passes all tests.
> >
> Which error?  I notice some changes related to minimizing performance
> impact, but I'm not seeing what I did wrong.  Just curious for my own
> benefit on that one.

During combination of ZEND_FETCH, ZEND_FETCH_OBJ into one ZEND_FETCH_OBJ
with IS_CV you didn't check for first check type. So you could combine fetch
of static (or global) variable into CV. You can just run "make test" with
your and my patch and see difference.

> > BTW I am against this patch.
> >
> Fair enough.
>
> > 1) It adds sloppily built functionality into the core
> Well, you sure know how to sweet-talk a gal.

My English isn't good and I spent some time with dictionary to write that I
think :)
Really, I think that you use the most complicated way to solve HTTP input
encoding problem.

May be we no longer need for autoglobals at all and can use simple functions
like http_get_var($name).

Thanks. Dmitry.

Pierre Joye

19 years ago

On 1/27/07, Dmitry Stogov <dmitry@zend.com> wrote:

> Hi Sara,
>
> > -----Original Message-----
> > From: Sara Golemon [mailto:pollita@php.net]
> > Sent: Friday, January 26, 2007 8:11 PM
> > To: Dmitry Stogov
> > Cc: internals@lists.php.net; 'Andi Gutmans'; 'Andrei
> > Zmievski'; Zeev Suraski; Stanislav Malyshev
> > Subject: Re: [PHP-DEV] Runtime-JIT, the whole enchilada
> >
> >
> > > I fixed one error in your patch, so now it passes all tests.
> > >
> > Which error?  I notice some changes related to minimizing performance
> > impact, but I'm not seeing what I did wrong.  Just curious for my own
> > benefit on that one.
>
> During combination of ZEND_FETCH, ZEND_FETCH_OBJ into one ZEND_FETCH_OBJ
> with IS_CV you didn't check for first check type. So you could combine fetch
> of static (or global) variable into CV. You can just run "make test" with
> your and my patch and see difference.
>
> > > BTW I am against this patch.
> > >
> > Fair enough.
> >
> > > 1) It adds sloppily built functionality into the core
> > Well, you sure know how to sweet-talk a gal.
>
> My English isn't good and I spent some time with dictionary to write that I
> think :)
> Really, I think that you use the most complicated way to solve HTTP input
> encoding problem.
>
> May be we no longer need for autoglobals at all and can use simple functions
> like http_get_var($name).

That's somehow my thoughts too. My initial proposal was to have a
simple JIT for GPC at runtime (which is not that hard to implement but
not easy either ;) and then use filter (as it exists for this exact
purpose!). Another approach I really like is the cgi object, see perl
or python for nice implementations examples.

--Pierre
ps: we are running in circle, I said that already many times already...

Andrei Zmievski

19 years ago

Good luck trying to retrain millions of programmers to use a CGI
object or a function to retrieve GPC values. Really, how much of a
performance hit is Sara's patch going to be, compared to other things
in PHP 6?

-Andrei

On Jan 26, 2007, at 3:38 PM, Pierre wrote:

Pierre Joye

19 years ago

Hello Andrei,

On 1/27/07, Andrei Zmievski <andrei@gravitonic.com> wrote:

> Good luck trying to retrain millions of programmers to use a CGI
> object or a function to retrieve GPC values.

You will be surprised, really :)

> Really, how much of a
> performance hit is Sara's patch going to be, compared to other things
> in PHP 6?

It is to early to argue about performance problems in my humble
opinion. My point was more about the complexity. That's why I prefer
the other solution which simply moves the JIT to runtime without other
changes but a function to get the encoding error.

To be honest, I do not understand the resistance to commit
experimental code in head. For what really matters, we need a working
solution. It does not need to be perfert, it needs to work (more or
less). We still have plenty of time to improve it until 6 is out (not
going to happen in the next months anyway).

--Pierre

Dmitry Stogov

19 years ago

> -----Original Message-----
> From: Pierre [mailto:pierre.php@gmail.com]
> Sent: Saturday, January 27, 2007 2:16 PM
> To: Andrei Zmievski
> Cc: Dmitry Stogov; Sara Golemon; internals@lists.php.net;
> Andi Gutmans; Zeev Suraski; Stanislav Malyshev
> Subject: Re: [PHP-DEV] Runtime-JIT, the whole enchilada
>
>
> Hello Andrei,
>
> On 1/27/07, Andrei Zmievski <andrei@gravitonic.com> wrote:
> > Good luck trying to retrain millions of programmers to use a CGI
> > object or a function to retrieve GPC values.
>
> You will be surprised, really :)
>
> > Really, how much of a
> > performance hit is Sara's patch going to be, compared to
> other things
> > in PHP 6?
>
> It is to early to argue about performance problems in my
> humble opinion. My point was more about the complexity.

Agree 100%. The slowdown is not large (may be 1%), but internal dependencies
are to complex.
This solution may be similar to "indirect modification of overloaded
properties" or "return by reference", then mistakes give hundreds of bug
reports and takes years for completely fix.

> That's why I prefer the other solution which simply moves the
> JIT to runtime without other changes but a function to get
> the encoding error.
>
> To be honest, I do not understand the resistance to commit
> experimental code in head. For what really matters, we need a
> working solution. It does not need to be perfert, it needs to
> work (more or less). We still have plenty of time to improve
> it until 6 is out (not going to happen in the next months anyway).

I cannot restrict commits, I only tell my opinion.
If the patch will be fixed for opcode caches, and most of developers will
vote for it, it can go.

Dmitry.

Pierre Joye

19 years ago

On 1/27/07, Dmitry Stogov <dmitry@zend.com> wrote:

> > -----Original Message-----
> > From: Pierre [mailto:pierre.php@gmail.com]
> > Sent: Saturday, January 27, 2007 2:16 PM
> > To: Andrei Zmievski
> > Cc: Dmitry Stogov; Sara Golemon; internals@lists.php.net;
> > Andi Gutmans; Zeev Suraski; Stanislav Malyshev
> > Subject: Re: [PHP-DEV] Runtime-JIT, the whole enchilada
> >
> >
> > Hello Andrei,
> >
> > On 1/27/07, Andrei Zmievski <andrei@gravitonic.com> wrote:
> > > Good luck trying to retrain millions of programmers to use a CGI
> > > object or a function to retrieve GPC values.
> >
> > You will be surprised, really :)
> >
> > > Really, how much of a
> > > performance hit is Sara's patch going to be, compared to
> > other things
> > > in PHP 6?
> >
> > It is to early to argue about performance problems in my
> > humble opinion. My point was more about the complexity.
>
> Agree 100%. The slowdown is not large (may be 1%), but internal dependencies
> are to complex.
> This solution may be similar to "indirect modification of overloaded
> properties" or "return by reference", then mistakes give hundreds of bug
> reports and takes years for completely fix.
>
> > That's why I prefer the other solution which simply moves the
> > JIT to runtime without other changes but a function to get
> > the encoding error.
> >
> > To be honest, I do not understand the resistance to commit
> > experimental code in head. For what really matters, we need a
> > working solution. It does not need to be perfert, it needs to
> > work (more or less). We still have plenty of time to improve
> > it until 6 is out (not going to happen in the next months anyway).
>
> I cannot restrict commits, I only tell my opinion.
> If the patch will be fixed for opcode caches, and most of developers will
> vote for it, it can go.

Oh! It was not a +1 from me for this patch, I still prefer my simpler
solution (which has to be written ;). However, as a temporary/test
code Sara's patch can do it for now no?

Stanislav Malyshev

19 years ago

> Oh! It was not a +1 from me for this patch, I still prefer my simpler
> solution (which has to be written ;). However, as a temporary/test
> code Sara's patch can do it for now no?

What's the reason to commit "temporary test code" to CVS? If one wants
to play with it, one can always apply a patch. There's no reason for the
code to be in the CVS unless you mean it to stay.

--
Stanislav Malyshev, Zend Products Engineer
stas@zend.com  http://www.zend.com/

Andi Gutmans

19 years ago

Sorry for the spam (I'm a bit behind).
I agree 100% with Dmitry.
Guys (n'girls), I don't know if you've realized but the engine has become much involved. We've added a lot of new features in PHP 5
and beyond, Unicode has muliplied that by 2, we added some performance features and we've just more or less managed to solve some of
the ugly issues with return by reference and overloaded properties. I think we need to move into a state of mind now that adding
additional implementation complexity is a last and not a first resort. Otherwise it'll end up being impossible to maintain all the
interdependencies. I said the same with past features during PHP 5 where I warned that they might introduce bugs in other areas but
it's hard to forsee. No one believed me because I couldn't give a concrete enough example and we had quite a few of those bugs
afterwards.
As some point the platform has to be the one that grows and not the language. So if we need to evolve it to have a different way of
fetching input variables that's what we should do. You don't see other languages change every day because of a higher level
requirement. And have no doubt, PHP 6 will take a long time to truly stabilize and make consistent because there've been huge
changes.

Andi

Pierre Joye

19 years ago

Hi Andi,

On 1/27/07, Andi Gutmans <andi@zend.com> wrote:

> s during PHP 5 where I warned that they might introduce bugs in other
> areas but it's hard to forsee. No one believed me because I couldn't
> give a concrete enough example and we had quite a few of those bugs
> afterwards.

Given the state of PHP5 when it was released, everyone sane should
believe (I would say trust) you, even if you were the one pushing php5
final release :)

> As some point the platform has to be the one that grows and
> not the language. So if we need to evolve it to have a different way of
> fetching input variables that's what we should do.

Exactly my point (again :-/), it is my goal for filter. That's also
why I asked you (I still wait the answer by the way) earlier this year
what are your needs about filters and what do you mean in your post,
as you were saying that we did not do a good job.

I, and I'm the only one in the active filter maintainers, changed my
mind (more open) and I really think that we should provide a OO way to
fetch the inputs. It can be type oriented (::getInteger()), variable
oriented (::get($name), and/or both. But the needs are definitively
here and I will do my best to push that in (a cgi-like object is one
solution).

> You don't see other languages change every day because of a higher level
> requirement. And have no doubt, PHP 6 will take a long time to truly stabilize and make consistent because there've been huge
> changes.

I 100% agree. No matter how much efforts are pushed into its
development, I cannot imagine a first stable release in a near future,
next year at best. By stable I mean a release not marked RC and
something with at least a beta quality and not alpha like 5.0.0.
Nobody will expect 6.0.0 to be perfect, but we should not make the
same mistake again.

Regards,
--Pierre

Andi Gutmans

19 years ago

Btw, having a request object was one of the #1 requests in framework :) People actually really like encapsulating this because it
also makes unit testing easier down the road...
Just mentioning this because I don't think we should be too set with our ways esp. for people who need to accomplish more
functionality.
Andi

Sara Golemon

19 years ago

> Btw, having a request object was one of the #1 requests in framework

:) People actually really like encapsulating this because it

> also makes unit testing easier down the road...
> Just mentioning this because I don't think we should be too set with

our ways esp. for people who need to accomplish more

> functionality.
>

For the record, I *do* prefer the simplicity of implementation of going
with a request object, and I *personally* don't see it as a
show-stopping BC break.  Then again, I didn't see that fgets() change as
show-stopping either.

So let's start back from square one.  How about a fresh round of
discussion on the request object approach, in psuedo-code:

class PHPGetObject implements ArrayAccess
{
private $decoded = array();

public function __offset_get($varname)
{
if (!isset($this->decoded[$varname])) {
$val = http_decode_get($varname);
$this->decoded[$varname] = $val;
}

return $this->decoded[$varname];
}
/* plus set,isset,unset of course */
/* Probably need an iterator too */
}


Pros: Fast, (mostly) clean, and cheap.
Cons: Breaks the following BC bahaviors:
* is_array($_GET) === false
* is_object($_GET) === true
* Referenceishness:
$get = $_GET;
$get['foo'] = 'bar';
var_dump($_GET['foo']); /* 'bar' */

My vote: +1 as I don't think the Cons are that serious.

-Sara

Andi Gutmans

19 years ago

Thanks Sara for the additional insight. Give me a few days to look into this further and catch the various people in person.
Unfortunately I'm going to be at a four day offsite Monday-Thursday but I'll still try my best to touch base and try and find some
agreed upon solution.
Right now, I would lean towards something like Sara suggested in this email (overloaded object), possibly improving some overloading
functionality on top. But I need to dive deeper into this. I have really been behind all this stuff due to a heavy work load.

Andi

Pierre Joye

19 years ago

On 1/27/07, Sara Golemon <pollita@php.net> wrote:

>  > Btw, having a request object was one of the #1 requests in framework
> :) People actually really like encapsulating this because it
>  > also makes unit testing easier down the road...
>  > Just mentioning this because I don't think we should be too set with
> our ways esp. for people who need to accomplish more
>  > functionality.
>  >
> For the record, I *do* prefer the simplicity of implementation of going
> with a request object, and I *personally* don't see it as a
> show-stopping BC break.  Then again, I didn't see that fgets() change as
> show-stopping either.
>
> So let's start back from square one.  How about a fresh round of
> discussion on the request object approach, in psuedo-code:
>
> class PHPGetObject implements ArrayAccess
> {
>      private $decoded = array();
>
>      public function __offset_get($varname)
>      {
>          if (!isset($this->decoded[$varname])) {
>              $val = http_decode_get($varname);
>              $this->decoded[$varname] = $val;
>          }
>
>          return $this->decoded[$varname];
>      }
>      /* plus set,isset,unset of course */
>      /* Probably need an iterator too */
> }
>
>
> Pros: Fast, (mostly) clean, and cheap.
> Cons: Breaks the following BC bahaviors:
>    * is_array($_GET) === false
>    * is_object($_GET) === true
>    * Referenceishness:
>       $get = $_GET;
>      $get['foo'] = 'bar';
>      var_dump($_GET['foo']); /* 'bar' */
>
> My vote: +1 as I don't think the Cons are that serious.

That's not what I had in mind while talking about an object.
ArrayAccess was never an option for the exact reasons you quoted in
the cons. However if this problem can get fixed at the engine level,
why not.

--Pierre

Dmitry Stogov

19 years ago

+1

I like this solution and I don't think that BC break is important for many
applications.
Not a lot of them use is_array($GET), and I believe no one use
is_object($_GET).
$get = $_GET may be a problem, but moving from PHP5 to PHP6 won't easy in
any case.

Dmitry.

Richard Lynch

19 years ago

On Sat, January 27, 2007 5:16 am, Pierre wrote:

> On 1/27/07, Andrei Zmievski <andrei@gravitonic.com> wrote:
>> Good luck trying to retrain millions of programmers to use a CGI
>> object or a function to retrieve GPC values.
>
> You will be surprised, really :)

I doubt it.

We haven't even gotten everybody to get rid of register_globals and
switch to $_GET['foo'] instead of $foo yet.

The register_globals change at least had a clear Security reasoning
behind it, once one understood the problem.
[Though I confess I was in denial for ages since I always initialize
variables.]

If we change this around yet again, it will be disastrous, imho.

--
Some people have a "gift" link here.
Know what I want?
I want you to buy a CD from some starving artist.
http://cdbaby.com/browse/from/lynch
Yeah, I get a buck. So?

Pierre Joye

19 years ago

On 1/29/07, Richard Lynch <ceo@l-i-e.com> wrote:

> On Sat, January 27, 2007 5:16 am, Pierre wrote:
> > On 1/27/07, Andrei Zmievski <andrei@gravitonic.com> wrote:
> >> Good luck trying to retrain millions of programmers to use a CGI
> >> object or a function to retrieve GPC values.
> >
> > You will be surprised, really :)
>
> I doubt it.
>
> We haven't even gotten everybody to get rid of register_globals and
> switch to $_GET['foo'] instead of $foo yet.
>
> The register_globals change at least had a clear Security reasoning
> behind it, once one understood the problem.
> [Though I confess I was in denial for ages since I always initialize
> variables.]
>
> If we change this around yet again, it will be disastrous, imho.

Please, who said we will *drop* the GPC array? That's exactly what I
try to avoid. What I say is to __add__ something like that,  not to
replace gpc.

--Pierre

Andrei Zmievski

19 years ago

Of course we won't drop it, but it would contain raw binary data (as
opposed to the rest of the PHP environment in unicode semantics mode).
IMO, it is inconsistent and confusing to the end user.

-A

On Jan 29, 2007, at 3:20 PM, Pierre wrote: