From 0989f65a1acf23064d7bb25e7704202ea93f708c Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Sun, 17 Nov 2019 11:43:24 -0500 Subject: [PATCH] Optimize array_key_exists/in_array for empty array Make opcache replace the result with false if the array argument is known to be empty. This may be useful when a codebase has placeholders, e.g. `if (!in_array($method, self::ALLOWED_METHODS)) { return; }` In zend_inference.c: In php 8, array_key_exists will throw a TypeError instead of returning null. I didn't see any discussion of this optimization (for/against) after a quick search on github, e.g. GH-3360 Potential future optimizations: - convert `in_array($needle, ['only one element'], true)` to `===`? (or `==` for strict=false) - When the number of elements is less than 4, switch to looping instead of hash lookup. (exact threshold for better performance to be determined) Also support looping for `in_array($value, [false, 'str', 2.5], true/false)` --- Zend/Optimizer/dfa_pass.c | 67 ++++++++++++------- Zend/Optimizer/sccp.c | 18 +++++ ext/opcache/tests/array_key_exists_empty.phpt | 46 +++++++++++++ ext/opcache/tests/in_array_empty.phpt | 51 ++++++++++++++ 4 files changed, 158 insertions(+), 24 deletions(-) create mode 100644 ext/opcache/tests/array_key_exists_empty.phpt create mode 100644 ext/opcache/tests/in_array_empty.phpt diff --git a/Zend/Optimizer/dfa_pass.c b/Zend/Optimizer/dfa_pass.c index 114a1b8ce243a..e6d8d2701fa09 100644 --- a/Zend/Optimizer/dfa_pass.c +++ b/Zend/Optimizer/dfa_pass.c @@ -446,31 +446,50 @@ int zend_dfa_optimize_calls(zend_op_array *op_array, zend_ssa *ssa) uint32_t op_num = send_needly - op_array->opcodes; zend_ssa_op *ssa_op = ssa->ops + op_num; - if (ssa_op->op1_use >= 0) { - /* Reconstruct SSA */ - int var_num = ssa_op->op1_use; - zend_ssa_var *var = ssa->vars + var_num; - - ZEND_ASSERT(ssa_op->op1_def < 0); - zend_ssa_unlink_use_chain(ssa, op_num, ssa_op->op1_use); - ssa_op->op1_use = -1; - ssa_op->op1_use_chain = -1; - op_num = call_info->caller_call_opline - op_array->opcodes; - ssa_op = ssa->ops + op_num; - ssa_op->op1_use = var_num; - ssa_op->op1_use_chain = var->use_chain; - var->use_chain = op_num; - } - - ZVAL_ARR(&tmp, dst); + if (zend_hash_num_elements(src) == 0 && + !(ssa->var_info[ssa_op->op1_use].type & MAY_BE_UNDEF)) { + if (ssa_op->op1_use >= 0) { + /* Reconstruct SSA - the needle is no longer used by any part of the call */ + ZEND_ASSERT(ssa_op->op1_def < 0); + zend_ssa_unlink_use_chain(ssa, op_num, ssa_op->op1_use); + ssa_op->op1_use = -1; + ssa_op->op1_use_chain = -1; + } + /* TODO remove needle from the uses of ssa graph? */ + ZVAL_FALSE(&tmp); + zend_array_destroy(dst); + + call_info->caller_call_opline->opcode = ZEND_QM_ASSIGN; + call_info->caller_call_opline->extended_value = 0; + call_info->caller_call_opline->op1_type = IS_CONST; + call_info->caller_call_opline->op1.constant = zend_optimizer_add_literal(op_array, &tmp); + call_info->caller_call_opline->op2_type = IS_UNUSED; + } else { + if (ssa_op->op1_use >= 0) { + /* Reconstruct SSA - the needle is now used by the ZEND_IN_ARRAY opline */ + int var_num = ssa_op->op1_use; + zend_ssa_var *var = ssa->vars + var_num; - /* Update opcode */ - call_info->caller_call_opline->opcode = ZEND_IN_ARRAY; - call_info->caller_call_opline->extended_value = strict; - call_info->caller_call_opline->op1_type = send_needly->op1_type; - call_info->caller_call_opline->op1.num = send_needly->op1.num; - call_info->caller_call_opline->op2_type = IS_CONST; - call_info->caller_call_opline->op2.constant = zend_optimizer_add_literal(op_array, &tmp); + ZEND_ASSERT(ssa_op->op1_def < 0); + zend_ssa_unlink_use_chain(ssa, op_num, ssa_op->op1_use); + ssa_op->op1_use = -1; + ssa_op->op1_use_chain = -1; + op_num = call_info->caller_call_opline - op_array->opcodes; + ssa_op = ssa->ops + op_num; + ssa_op->op1_use = var_num; + ssa_op->op1_use_chain = var->use_chain; + var->use_chain = op_num; + } + ZVAL_ARR(&tmp, dst); + + /* Update opcode */ + call_info->caller_call_opline->opcode = ZEND_IN_ARRAY; + call_info->caller_call_opline->extended_value = strict; + call_info->caller_call_opline->op1_type = send_needly->op1_type; + call_info->caller_call_opline->op1.num = send_needly->op1.num; + call_info->caller_call_opline->op2_type = IS_CONST; + call_info->caller_call_opline->op2.constant = zend_optimizer_add_literal(op_array, &tmp); + } if (call_info->caller_init_opline->extended_value == 3) { MAKE_NOP(call_info->caller_call_opline - 1); } diff --git a/Zend/Optimizer/sccp.c b/Zend/Optimizer/sccp.c index 56c4d24d9bfe8..f2da1ff67a1a7 100644 --- a/Zend/Optimizer/sccp.c +++ b/Zend/Optimizer/sccp.c @@ -696,6 +696,10 @@ static inline zend_result ct_eval_in_array(zval *result, uint32_t extended_value return FAILURE; } ht = Z_ARRVAL_P(op2); + if (zend_hash_num_elements(ht) == 0) { + ZVAL_FALSE(result); + return SUCCESS; + } if (EXPECTED(Z_TYPE_P(op1) == IS_STRING)) { res = zend_hash_exists(ht, Z_STR_P(op1)); } else if (extended_value) { @@ -1211,6 +1215,20 @@ static void sccp_visit_instr(scdf_ctx *scdf, zend_op *opline, zend_ssa_op *ssa_o ssa_op++; SET_RESULT_BOT(op1); break; + case ZEND_ARRAY_KEY_EXISTS: + if (ctx->scdf.ssa->var_info[ssa_op->op1_use].type & ~(MAY_BE_NULL|MAY_BE_FALSE|MAY_BE_TRUE|MAY_BE_LONG|MAY_BE_DOUBLE|MAY_BE_STRING)) { + /* Skip needles that could cause TypeError in array_key_exists */ + break; + } + ZEND_FALLTHROUGH; + case ZEND_IN_ARRAY: + SKIP_IF_TOP(op2); + if (Z_TYPE_P(op2) == IS_ARRAY && zend_hash_num_elements(Z_ARRVAL_P(op2)) == 0) { + ZVAL_FALSE(&zv); + SET_RESULT(result, &zv); + return; + } + break; } if ((op1 && IS_BOT(op1)) || (op2 && IS_BOT(op2))) { diff --git a/ext/opcache/tests/array_key_exists_empty.phpt b/ext/opcache/tests/array_key_exists_empty.phpt new file mode 100644 index 0000000000000..00465a866a081 --- /dev/null +++ b/ext/opcache/tests/array_key_exists_empty.phpt @@ -0,0 +1,46 @@ +--TEST-- +array_key_exists() on known empty array +--EXTENSIONS-- +opcache +--FILE-- + true]); + $z2 = array_key_exists($x, [2 => true]); + $w = array_key_exists('literal', self::EMPTY_ARRAY); + echo helper($y); + echo helper($z); + echo helper($w); + echo helper($z1); + echo helper($z2); + $unusedVar = array_key_exists('unused', $arr); + if (array_key_exists(printf("Should get called\n"), self::EMPTY_ARRAY)) { + echo "Impossible\n"; + } + $v = array_key_exists($arr, self::EMPTY_ARRAY); + } +} +try { + ExampleArrayKeyExists::test(1,[2]); +} catch (TypeError $e) { + printf("%s at line %d\n", $e->getMessage(), $e->getLine()); +} +?> +--EXPECTF-- +Warning: Undefined variable $undef in %s on line 10 +bool(false) +bool(false) +bool(false) +bool(true) +bool(false) +Should get called +Illegal offset type at line 24 diff --git a/ext/opcache/tests/in_array_empty.phpt b/ext/opcache/tests/in_array_empty.phpt new file mode 100644 index 0000000000000..cfa91ee7da6aa --- /dev/null +++ b/ext/opcache/tests/in_array_empty.phpt @@ -0,0 +1,51 @@ +--TEST-- +in_array() on known empty array +--EXTENSIONS-- +opcache +--FILE-- + +--EXPECTF-- +Warning: Undefined variable $undef in %s on line 11 +bool(false) +bool(false) +bool(false) +bool(false) +Results for non-empty arrays +bool(true) +bool(false) +bool(true) +bool(false) +Should get called \ No newline at end of file