At first I don't understand why you are trying to deallocate variable.

+               if (CG(active_op_array)->T == (parent->u.var - 1)) {
+                       CG(active_op_array)->T--;
+               }

Isn't the same variable reused as result of ZEND_FETCH_DIM?

Yeah, I hastily recanted that about three minutes after my post... It was accidentally left in from a prior attempt. You're absolutely right that it doesn't belong there. Just my sloppiness in preparing the patch for posting.

The rest of patch seems proper, but I am not sure about place.
Why we optimize @$a['b'] but not @$a->b and @$a->b()?

I would prefer to find more general solution.

Well, we could do it in the end_variable_parse, but at that point we actually *do* have to erase an op and a temp var. Doing it at this stage prevents the extra op/tempvar from being allocated at all.

Looking at do_fetch_property, I notice that we're already doing this (after a fashion) for turning $this->prop into an immediate FETCH_OBJ (setting op1 to UNUSED to indicate $this). This approach really just builds on that existing solution.

I've updated the patch (adding FETCH_OBJ support and applying some concepts from do_fetch_property's implementation for $this optimization to the FETCH_DIM/OBJ CV implementations).

I suppose the one alternate approach we could take would be to make a macro/inline func out of this rewriting step so that the DIM/OBJ parsers can use the same code-base. I might even suggest we look at supporting op1->IS_UNUSED for $this when used with array access on objects (but that would require an update so that the opcode handler supports it as well).

-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 23 Jan 2007 09:21:41 -0000
@@ -492,6 +492,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 +525,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 +3283,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 +3316,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;
                }
        }
 

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to