Skip to content

Commit 631f856

Browse files
committed
Split out the TYPE_CHECK implementation for resources
- The old version is harder to reason about, due to having different handling for resources. Opcache has to check for MAY_BE_RESOURCE when transforming opcodes, JITing operations, etc. - Nikic suggested considering a separate opcode in phpGH-4906 Performance impact: no improvement/worsening, unless hypothetically if is_resource and is_int calls were to alternate. - I tried optimizations such as avoiding SAVE_OPLINE when possible in TYPE_CHECK. This somehow made performance worse (possibly due to larger code size). Making this COLD might make more sense - I doubt a typical application would be bottlenecked by is_resource. E.g. this case was unaffected for `loopnotnull_isnull(0, 30000000);` ```php function loopnotnull_isnull($a, $b) { $count = 0; for ($i = $a; $i < $b; $i++) { if (is_null($i)) { $count++; } } return $count; } ``` **NOTE: opcache.file_cache should be cleared by developers after this commit**
1 parent e208d23 commit 631f856

14 files changed

+721
-547
lines changed

‎Zend/zend_compile.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2078,6 +2078,7 @@ ZEND_API int zend_is_smart_branch(const zend_op *opline) /* {{{ */
20782078
case ZEND_ISSET_ISEMPTY_STATIC_PROP:
20792079
case ZEND_INSTANCEOF:
20802080
case ZEND_TYPE_CHECK:
2081+
case ZEND_IS_RESOURCE:
20812082
case ZEND_DEFINED:
20822083
case ZEND_IN_ARRAY:
20832084
case ZEND_ARRAY_KEY_EXISTS:
@@ -3409,6 +3410,20 @@ static int zend_compile_func_is_scalar(znode *result, zend_ast_list *args) /* {{
34093410
return SUCCESS;
34103411
}
34113412

3413+
int zend_compile_func_is_resource(znode *result, zend_ast_list *args) /* {{{ */
3414+
{
3415+
znode arg_node;
3416+
3417+
if (args->children != 1) {
3418+
return FAILURE;
3419+
}
3420+
3421+
zend_compile_expr(&arg_node, args->child[0]);
3422+
zend_emit_op_tmp(result, ZEND_IS_RESOURCE, &arg_node, NULL);
3423+
return SUCCESS;
3424+
}
3425+
/* }}} */
3426+
34123427
int zend_compile_func_cast(znode *result, zend_ast_list *args, uint32_t type) /* {{{ */
34133428
{
34143429
znode arg_node;
@@ -3926,7 +3941,7 @@ int zend_try_compile_special_func(znode *result, zend_string *lcname, zend_ast_l
39263941
} else if (zend_string_equals_literal(lcname, "is_object")) {
39273942
return zend_compile_func_typecheck(result, args, IS_OBJECT);
39283943
} else if (zend_string_equals_literal(lcname, "is_resource")) {
3929-
return zend_compile_func_typecheck(result, args, IS_RESOURCE);
3944+
return zend_compile_func_is_resource(result, args);
39303945
} else if (zend_string_equals_literal(lcname, "is_scalar")) {
39313946
return zend_compile_func_is_scalar(result, args);
39323947
} else if (zend_string_equals_literal(lcname, "boolval")) {

‎Zend/zend_vm_def.h

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7862,16 +7862,9 @@ ZEND_VM_HOT_NOCONST_HANDLER(123, ZEND_TYPE_CHECK, CONST|TMPVAR|CV, ANY, TYPE_MAS
78627862

78637863
value = GET_OP1_ZVAL_PTR_UNDEF(BP_VAR_R);
78647864
if ((opline->extended_value >> (uint32_t)Z_TYPE_P(value)) & 1) {
7865-
ZEND_VM_C_LABEL(type_check_resource):
7866-
if (opline->extended_value != MAY_BE_RESOURCE
7867-
|| EXPECTED(NULL != zend_rsrc_list_get_rsrc_type(Z_RES_P(value)))) {
7868-
result = 1;
7869-
}
7865+
result = 1;
78707866
} else if ((OP1_TYPE & (IS_CV|IS_VAR)) && Z_ISREF_P(value)) {
7871-
value = Z_REFVAL_P(value);
7872-
if ((opline->extended_value >> (uint32_t)Z_TYPE_P(value)) & 1) {
7873-
ZEND_VM_C_GOTO(type_check_resource);
7874-
}
7867+
result = (opline->extended_value >> (uint32_t)Z_TYPE_P(Z_REFVAL_P(value))) & 1;
78757868
} else if (OP1_TYPE == IS_CV && UNEXPECTED(Z_TYPE_P(value) == IS_UNDEF)) {
78767869
result = ((1 << IS_NULL) & opline->extended_value) != 0;
78777870
SAVE_OPLINE();
@@ -7890,6 +7883,38 @@ ZEND_VM_C_LABEL(type_check_resource):
78907883
}
78917884
}
78927885

7886+
ZEND_VM_HOT_NOCONST_HANDLER(195, ZEND_IS_RESOURCE, CONST|TMPVAR|CV, ANY, TYPE_MASK)
7887+
{
7888+
USE_OPLINE
7889+
zval *value;
7890+
7891+
value = GET_OP1_ZVAL_PTR_UNDEF(BP_VAR_R);
7892+
ZEND_VM_C_LABEL(type_check_resource):
7893+
if (EXPECTED(Z_TYPE_P(value) == IS_RESOURCE)) {
7894+
int result = EXPECTED(NULL != zend_rsrc_list_get_rsrc_type(Z_RES_P(value)));
7895+
SAVE_OPLINE();
7896+
FREE_OP1();
7897+
ZEND_VM_SMART_BRANCH(result, 1);
7898+
} else if ((OP1_TYPE & (IS_CV|IS_VAR)) && Z_ISREF_P(value)) {
7899+
value = Z_REFVAL_P(value);
7900+
if ((opline->extended_value >> (uint32_t)Z_TYPE_P(value)) & 1) {
7901+
ZEND_VM_C_GOTO(type_check_resource);
7902+
}
7903+
} else if (OP1_TYPE == IS_CV && UNEXPECTED(Z_TYPE_P(value) == IS_UNDEF)) {
7904+
SAVE_OPLINE();
7905+
ZVAL_UNDEFINED_OP1();
7906+
if (UNEXPECTED(EG(exception))) {
7907+
/* I'm not sure why ZEND_TYPE_CHECK did this. */
7908+
ZVAL_UNDEF(EX_VAR(opline->result.var));
7909+
HANDLE_EXCEPTION();
7910+
}
7911+
ZEND_VM_SMART_BRANCH(0, 0);
7912+
}
7913+
SAVE_OPLINE();
7914+
FREE_OP1();
7915+
ZEND_VM_SMART_BRANCH(0, 1);
7916+
}
7917+
78937918
ZEND_VM_HOT_HANDLER(122, ZEND_DEFINED, CONST, ANY, CACHE_SLOT)
78947919
{
78957920
USE_OPLINE

0 commit comments

Comments
 (0)