Hi,

Recently, I've found that OPcache optimizer misses a lot of abilities,
because it handles only one op_array at once. So it definitely can't
perform any inter-function optimizations (e.g. inlining).

Actually, it was not very difficult to switch to "script at once" approach.
The attached patch demonstrates it and adds per script constants
substitution explained in the following script

<?php
define("FOO", 1);
function foo() {
    echo FOO . "\n"; // optimizer will replace it with: echo "1\n";
}
?>

Of course, I ran the PHP test suite and it passed all the same tests.
Personally, I think it's safe to include this patch into 5.5 and make a
green light to some other advanced optimizations in 5.5. (e.g. conversion
INIT_FCALL_BY_NAME into DO_FCALL).

Any thoughts?

Thanks. Dmitry.
diff --git a/ext/opcache/Optimizer/pass1_5.c b/ext/opcache/Optimizer/pass1_5.c
index dc9e731..3a32970 100644
--- a/ext/opcache/Optimizer/pass1_5.c
+++ b/ext/opcache/Optimizer/pass1_5.c
@@ -10,6 +10,7 @@ if (ZEND_OPTIMIZER_PASS_1 & OPTIMIZATION_LEVEL) {
        int i = 0;
        zend_op *opline = op_array->opcodes;
        zend_op *end = opline + op_array->last;
+       zend_bool collect_constants = (op_array == &script->main_op_array);
 
        while (opline < end) {
                switch (opline->opcode) {
@@ -357,7 +358,9 @@ if (ZEND_OPTIMIZER_PASS_1 & OPTIMIZATION_LEVEL) {
                                zval c;
 
                                if 
(!zend_get_persistent_constant(Z_STRVAL(ZEND_OP2_LITERAL(opline)), 
Z_STRLEN(ZEND_OP2_LITERAL(opline)), &c, 1 TSRMLS_CC)) {
-                                       break;
+                                       if (!*constants || 
!zend_optimizer_get_collected_constant(*constants, &ZEND_OP2_LITERAL(opline), 
&c)) {
+                                               break;
+                                       }
                                }
                                literal_dtor(&ZEND_OP2_LITERAL(opline));
                                ZEND_OP1_TYPE(opline) = IS_CONST;
@@ -388,6 +391,71 @@ if (ZEND_OPTIMIZER_PASS_1 & OPTIMIZATION_LEVEL) {
                                }
                        }
                        break;
+               case ZEND_DO_FCALL:
+                       /* define("name", scalar); */
+                       if (collect_constants &&
+                           opline->extended_value == 2 &&
+                           ZEND_OP1_TYPE(opline) == IS_CONST &&
+                           Z_TYPE(ZEND_OP1_LITERAL(opline)) == IS_STRING &&
+                           Z_STRLEN(ZEND_OP1_LITERAL(opline)) == 
sizeof("define")-1 &&
+                           
zend_binary_strcasecmp(Z_STRVAL(ZEND_OP1_LITERAL(opline)), 
Z_STRLEN(ZEND_OP1_LITERAL(opline)), "define", sizeof("define")-1) == 0 &&
+                           (opline-1)->opcode == ZEND_SEND_VAL &&
+                           ZEND_OP1_TYPE(opline-1) == IS_CONST &&
+                           (Z_TYPE(ZEND_OP1_LITERAL(opline-1)) <= IS_BOOL ||
+                            Z_TYPE(ZEND_OP1_LITERAL(opline-1)) == IS_STRING) &&
+                           (opline-2)->opcode == ZEND_SEND_VAL &&
+                           ZEND_OP1_TYPE(opline-2) == IS_CONST &&
+                           Z_TYPE(ZEND_OP1_LITERAL(opline-2)) == IS_STRING) {
+                               zend_optimizer_collect_constant(constants, 
&ZEND_OP1_LITERAL(opline-2), &ZEND_OP1_LITERAL(opline-1));
+                       }
+                       break;
+#if ZEND_EXTENSION_API_NO > PHP_5_2_X_API_NO
+               case ZEND_DECLARE_CONST:
+                       if (collect_constants &&
+                           Z_TYPE(ZEND_OP1_LITERAL(opline)) == IS_STRING &&
+                           (Z_TYPE(ZEND_OP2_LITERAL(opline)) <= IS_BOOL ||
+                            Z_TYPE(ZEND_OP2_LITERAL(opline)) == IS_STRING)) {
+                               zend_optimizer_collect_constant(constants, 
&ZEND_OP1_LITERAL(opline), &ZEND_OP2_LITERAL(opline));
+                       }
+                       break;
+#endif
+
+               case ZEND_RETURN:
+#if ZEND_EXTENSION_API_NO > PHP_5_3_X_API_NO
+               case ZEND_RETURN_BY_REF:
+#endif
+#if ZEND_EXTENSION_API_NO > PHP_5_4_X_API_NO
+               case ZEND_GENERATOR_RETURN:
+#endif
+               case ZEND_EXIT:
+               case ZEND_THROW:
+               case ZEND_CATCH:
+               case ZEND_BRK:
+               case ZEND_CONT:
+#if ZEND_EXTENSION_API_NO >= PHP_5_3_X_API_NO
+               case ZEND_GOTO:
+#endif
+#if ZEND_EXTENSION_API_NO > PHP_5_4_X_API_NO
+               case ZEND_FAST_CALL:
+               case ZEND_FAST_RET:
+#endif
+               case ZEND_JMP:
+               case ZEND_JMPZNZ:
+               case ZEND_JMPZ:
+               case ZEND_JMPNZ:
+               case ZEND_JMPZ_EX:
+               case ZEND_JMPNZ_EX:
+               case ZEND_FE_RESET:
+               case ZEND_FE_FETCH:
+               case ZEND_NEW:
+#if ZEND_EXTENSION_API_NO >= PHP_5_3_X_API_NO
+               case ZEND_JMP_SET:
+#endif
+#if ZEND_EXTENSION_API_NO > PHP_5_3_X_API_NO
+               case ZEND_JMP_SET_VAR:
+#endif
+                       collect_constants = 0;
+                       break;
                }
                opline++;
                i++;
diff --git a/ext/opcache/Optimizer/zend_optimizer.c 
b/ext/opcache/Optimizer/zend_optimizer.c
index b574ecc..92f5f4a 100644
--- a/ext/opcache/Optimizer/zend_optimizer.c
+++ b/ext/opcache/Optimizer/zend_optimizer.c
@@ -24,28 +24,47 @@
 #include "zend_API.h"
 #include "zend_constants.h"
 #include "zend_execute.h"
+#include "zend_vm.h"
 
 #define OPTIMIZATION_LEVEL \
        ZCG(accel_directives).optimization_level
 
+static void zend_optimizer_zval_dtor_wrapper(zval *zvalue)
+{
+       zval_dtor(zvalue);
+}
+
+static void zend_optimizer_collect_constant(HashTable **constants, zval *name, 
zval* value)
+{
+       zval val;
+
+       if (!*constants) {
+               *constants = emalloc(sizeof(HashTable));
+               zend_hash_init(*constants, 16, NULL, (void (*)(void 
*))zend_optimizer_zval_dtor_wrapper, 0);
+       }
+       val = *value;
+       zval_copy_ctor(&val);
+       zend_hash_add(*constants, Z_STRVAL_P(name), Z_STRLEN_P(name)+1, 
(void**)&val, sizeof(zval), NULL);
+}
+
+static int zend_optimizer_get_collected_constant(HashTable *constants, zval 
*name, zval* value)
+{
+       zval *val;
+
+       if (zend_hash_find(constants, Z_STRVAL_P(name), Z_STRLEN_P(name)+1, 
(void**)&val) == SUCCESS) {
+               *value = *val;
+               zval_copy_ctor(value);
+               return 1;
+       }
+       return 0;
+}
+
 #if ZEND_EXTENSION_API_NO > PHP_5_3_X_API_NO
 int zend_optimizer_add_literal(zend_op_array *op_array, const zval *zv 
TSRMLS_DC)
 {
        int i = op_array->last_literal;
        op_array->last_literal++;
-#if ZEND_EXTENSION_API_NO > PHP_5_3_X_API_NO
-       {
-               if (i >= CG(context).literals_size) {
-                       CG(context).literals_size += 16; /* FIXME */
-                       op_array->literals = 
(zend_literal*)erealloc(op_array->literals, CG(context).literals_size * 
sizeof(zend_literal));
-               }
-       }
-#else
-       if (i >= op_array->size_literal) {
-               op_array->size_literal += 16; /* FIXME */
-               op_array->literals = 
(zend_literal*)erealloc(op_array->literals, op_array->size_literal * 
sizeof(zend_literal));
-       }
-#endif
+       op_array->literals = (zend_literal*)erealloc(op_array->literals, 
op_array->last_literal * sizeof(zend_literal));
        op_array->literals[i].constant = *zv;
        Z_SET_REFCOUNT(op_array->literals[i].constant, 2);
        Z_SET_ISREF(op_array->literals[i].constant);
@@ -92,7 +111,9 @@ int zend_optimizer_add_literal(zend_op_array *op_array, 
const zval *zv TSRMLS_DC
 #include "Optimizer/block_pass.c"
 #include "Optimizer/optimize_temp_vars_5.c"
 
-void zend_optimizer(zend_op_array *op_array TSRMLS_DC)
+static void zend_optimize(zend_op_array           *op_array,
+                          zend_persistent_script  *script,
+                          HashTable              **constants TSRMLS_DC)
 {
        if (op_array->type == ZEND_EVAL_CODE ||
            (op_array->fn_flags & ZEND_ACC_INTERACTIVE)) {
@@ -137,3 +158,133 @@ void zend_optimizer(zend_op_array *op_array TSRMLS_DC)
         */
 #include "Optimizer/pass10.c"
 }
+
+static void zend_accel_optimize(zend_op_array           *op_array,
+                                zend_persistent_script  *script,
+                                HashTable              **constants TSRMLS_DC)
+{
+       zend_op *opline, *end;
+
+       /* Revert pass_two() */
+       opline = op_array->opcodes;
+       end = opline + op_array->last;
+       while (opline < end) {
+#if ZEND_EXTENSION_API_NO > PHP_5_3_X_API_NO
+               if (opline->op1_type == IS_CONST) {
+                       opline->op1.constant = opline->op1.literal - 
op_array->literals;
+               }
+               if (opline->op2_type == IS_CONST) {
+                       opline->op2.constant = opline->op2.literal - 
op_array->literals;
+               }
+#endif
+               switch (opline->opcode) {
+                       case ZEND_JMP:
+#if ZEND_EXTENSION_API_NO > PHP_5_2_X_API_NO
+                       case ZEND_GOTO:
+#endif
+#if ZEND_EXTENSION_API_NO > PHP_5_4_X_API_NO
+                       case ZEND_FAST_CALL:
+#endif
+                               ZEND_OP1(opline).opline_num = 
ZEND_OP1(opline).jmp_addr - op_array->opcodes;
+                               break;
+                       case ZEND_JMPZ:
+                       case ZEND_JMPNZ:
+                       case ZEND_JMPZ_EX:
+                       case ZEND_JMPNZ_EX:
+#if ZEND_EXTENSION_API_NO > PHP_5_2_X_API_NO
+                       case ZEND_JMP_SET:
+#endif
+#if ZEND_EXTENSION_API_NO > PHP_5_3_X_API_NO
+                       case ZEND_JMP_SET_VAR:
+#endif
+                               ZEND_OP2(opline).opline_num = 
ZEND_OP2(opline).jmp_addr - op_array->opcodes;
+                               break;
+               }
+               opline++;
+       }
+
+       /* Do actual optimizations */
+       zend_optimize(op_array, script, constants TSRMLS_CC);   
+       
+       /* Redo pass_two() */
+       opline = op_array->opcodes;
+       end = opline + op_array->last;
+       while (opline < end) {
+#if ZEND_EXTENSION_API_NO > PHP_5_3_X_API_NO
+               if (opline->op1_type == IS_CONST) {
+                       opline->op1.zv = 
&op_array->literals[opline->op1.constant].constant;
+               }
+               if (opline->op2_type == IS_CONST) {
+                       opline->op2.zv = 
&op_array->literals[opline->op2.constant].constant;
+               }
+#endif
+               switch (opline->opcode) {
+                       case ZEND_JMP:
+#if ZEND_EXTENSION_API_NO > PHP_5_2_X_API_NO
+                       case ZEND_GOTO:
+#endif
+#if ZEND_EXTENSION_API_NO > PHP_5_4_X_API_NO
+                       case ZEND_FAST_CALL:
+#endif
+                               ZEND_OP1(opline).jmp_addr = 
&op_array->opcodes[ZEND_OP1(opline).opline_num];
+                               break;
+                       case ZEND_JMPZ:
+                       case ZEND_JMPNZ:
+                       case ZEND_JMPZ_EX:
+                       case ZEND_JMPNZ_EX:
+#if ZEND_EXTENSION_API_NO > PHP_5_2_X_API_NO
+                       case ZEND_JMP_SET:
+#endif
+#if ZEND_EXTENSION_API_NO > PHP_5_3_X_API_NO
+                       case ZEND_JMP_SET_VAR:
+#endif
+                               ZEND_OP2(opline).jmp_addr = 
&op_array->opcodes[ZEND_OP2(opline).opline_num];
+                               break;
+               }
+               ZEND_VM_SET_OPCODE_HANDLER(opline);
+               opline++;
+       }
+}
+
+int zend_accel_script_optimize(zend_persistent_script *script TSRMLS_DC)
+{
+       Bucket *p, *q;
+       HashTable *constants = NULL;
+
+       zend_accel_optimize(&script->main_op_array, script, &constants 
TSRMLS_CC);
+
+       p = script->function_table.pListHead;
+       while (p) {
+               zend_op_array *op_array = (zend_op_array*)p->pData;
+               zend_accel_optimize(op_array, script, &constants TSRMLS_CC);
+               p = p->pListNext;
+       }
+
+       p = script->class_table.pListHead;
+       while (p) {
+               zend_class_entry *ce = (zend_class_entry*)p->pDataPtr;
+               q = ce->function_table.pListHead;
+               while (q) {
+                       zend_op_array *op_array = (zend_op_array*)q->pData;
+                       if (op_array->scope == ce) {
+                               zend_accel_optimize(op_array, script, 
&constants TSRMLS_CC);
+                       } else if (op_array->type == ZEND_USER_FUNCTION) {
+                               zend_op_array *orig_op_array;
+                               if 
(zend_hash_find(&op_array->scope->function_table, q->arKey, q->nKeyLength, 
(void**)&orig_op_array) == SUCCESS) {
+                                       HashTable *ht = 
op_array->static_variables;
+                                       *op_array = *orig_op_array;
+                                       op_array->static_variables = ht;
+                               }
+                       }
+                       q = q->pListNext;
+               }
+               p = p->pListNext;
+       }
+
+       if (constants) {
+               zend_hash_destroy(constants);
+               efree(constants);
+       }
+
+       return 1;
+}
diff --git a/ext/opcache/Optimizer/zend_optimizer.h 
b/ext/opcache/Optimizer/zend_optimizer.h
index 98275a2..1775994 100644
--- a/ext/opcache/Optimizer/zend_optimizer.h
+++ b/ext/opcache/Optimizer/zend_optimizer.h
@@ -44,6 +44,4 @@
 
 #define DEFAULT_OPTIMIZATION_LEVEL  "0xFFFFFFFF"
 
-void zend_optimizer(zend_op_array *op_array TSRMLS_DC);
-
 #endif
diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c
index b62f245..9a6c763 100644
--- a/ext/opcache/ZendAccelerator.c
+++ b/ext/opcache/ZendAccelerator.c
@@ -1126,6 +1126,10 @@ static zend_persistent_script 
*cache_script_in_shared_memory(zend_persistent_scr
                return new_persistent_script;
        }
 
+       if (!zend_accel_script_optimize(new_persistent_script TSRMLS_CC)) {
+               return new_persistent_script;
+       }
+
        /* exclusive lock */
        zend_shared_alloc_lock(TSRMLS_C);
 
@@ -2731,19 +2735,6 @@ void accelerator_shm_read_unlock(TSRMLS_D)
        }
 }
 
-static void accel_op_array_handler(zend_op_array *op_array)
-{
-       TSRMLS_FETCH();
-
-       if (ZCG(enabled) &&
-           accel_startup_ok &&
-           ZCSG(accelerator_enabled) &&
-           !ZSMMG(memory_exhausted) &&
-           !ZCSG(restart_pending)) {
-               zend_optimizer(op_array TSRMLS_CC);
-       }
-}
-
 ZEND_EXT_API zend_extension zend_extension_entry = {
        ACCELERATOR_PRODUCT_NAME,               /* name */
        ACCELERATOR_VERSION,                                    /* version */
@@ -2755,7 +2746,7 @@ ZEND_EXT_API zend_extension zend_extension_entry = {
        accel_activate,                                                 /* 
per-script activation */
        accel_deactivate,                                               /* 
per-script deactivation */
        NULL,                                                                   
/* message handler */
-       accel_op_array_handler,                                 /* op_array 
handler */
+       NULL,                                                                   
/* op_array handler */
        NULL,                                                                   
/* extended statement handler */
        NULL,                                                                   
/* extended fcall begin handler */
        NULL,                                                                   
/* extended fcall end handler */
diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h
index 85f9570..063c951 100644
--- a/ext/opcache/ZendAccelerator.h
+++ b/ext/opcache/ZendAccelerator.h
@@ -319,6 +319,7 @@ extern char *zps_api_failure_reason;
 void zend_accel_schedule_restart(zend_accel_restart_reason reason TSRMLS_DC);
 void zend_accel_schedule_restart_if_necessary(zend_accel_restart_reason reason 
TSRMLS_DC);
 int  zend_accel_invalidate(const char *filename, int filename_len, zend_bool 
force TSRMLS_DC);
+int  zend_accel_script_optimize(zend_persistent_script *persistent_script 
TSRMLS_DC);
 int  accelerator_shm_read_lock(TSRMLS_D);
 void accelerator_shm_read_unlock(TSRMLS_D);
 
-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to