From 9f410c2751f9bf71b0b2c7596967d1660d61d08c 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)` --- ext/opcache/Optimizer/dfa_pass.c | 31 +++++++---- ext/opcache/Optimizer/sccp.c | 17 +++++++ ext/opcache/Optimizer/zend_inference.c | 8 +-- ext/opcache/tests/array_key_exists_empty.phpt | 46 +++++++++++++++++ ext/opcache/tests/in_array_empty.phpt | 51 +++++++++++++++++++ 5 files changed, 136 insertions(+), 17 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/ext/opcache/Optimizer/dfa_pass.c b/ext/opcache/Optimizer/dfa_pass.c index 8802577154269..f857e7b01a202 100644 --- a/ext/opcache/Optimizer/dfa_pass.c +++ b/ext/opcache/Optimizer/dfa_pass.c @@ -448,16 +448,27 @@ int zend_dfa_optimize_calls(zend_op_array *op_array, zend_ssa *ssa) 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 (zend_hash_num_elements(src) == 0) { + /* 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 { + 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/ext/opcache/Optimizer/sccp.c b/ext/opcache/Optimizer/sccp.c index 57ae48b0217ca..a4ce6bf4d6ee0 100644 --- a/ext/opcache/Optimizer/sccp.c +++ b/ext/opcache/Optimizer/sccp.c @@ -728,6 +728,10 @@ static inline int ct_eval_in_array(zval *result, uint32_t extended_value, zval * 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) { @@ -1437,6 +1441,19 @@ 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; + } + 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/Optimizer/zend_inference.c b/ext/opcache/Optimizer/zend_inference.c index cb31ea21f2cba..975853a9eb559 100644 --- a/ext/opcache/Optimizer/zend_inference.c +++ b/ext/opcache/Optimizer/zend_inference.c @@ -2480,14 +2480,8 @@ static int zend_update_type_info(const zend_op_array *op_array, case ZEND_ISSET_ISEMPTY_STATIC_PROP: case ZEND_ASSERT_CHECK: case ZEND_IN_ARRAY: - UPDATE_SSA_TYPE(MAY_BE_FALSE|MAY_BE_TRUE, ssa_ops[i].result_def); - break; case ZEND_ARRAY_KEY_EXISTS: - tmp = MAY_BE_FALSE|MAY_BE_TRUE; - if (t2 & ((MAY_BE_ANY|MAY_BE_UNDEF) - (MAY_BE_ARRAY|MAY_BE_OBJECT))) { - tmp |= MAY_BE_NULL; - } - UPDATE_SSA_TYPE(tmp, ssa_ops[i].result_def); + UPDATE_SSA_TYPE(MAY_BE_FALSE|MAY_BE_TRUE, ssa_ops[i].result_def); break; case ZEND_CAST: if (ssa_ops[i].op1_def >= 0) { 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..d514cc2acd52a --- /dev/null +++ b/ext/opcache/tests/array_key_exists_empty.phpt @@ -0,0 +1,46 @@ +--TEST-- +array_key_exists() on known empty array +--SKIPIF-- + +--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..3ee41a7816b59 --- /dev/null +++ b/ext/opcache/tests/in_array_empty.phpt @@ -0,0 +1,51 @@ +--TEST-- +in_array() on known empty array +--SKIPIF-- + +--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