Skip to content

Also cache the computed stack size in INIT_FCALL_BY_NAME, speed up calling global functions in other files #7761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Also cache the computed stack size in INIT_FCALL_BY_NAME, speed up calling global functions in other files #7761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36ef123
Also cache the computed stack size in INIT_FCALL_BY_NAME
TysonAndre Dec 9, 2021
1c3dd19
Try to fix the build for jit
TysonAndre Dec 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Zend/Optimizer/compact_literals.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
opline->result.num = func_slot[opline->op2.constant];
} else {
opline->result.num = cache_size;
cache_size += sizeof(void *);
cache_size += opline->opcode == ZEND_INIT_FCALL_BY_NAME ? 2 * sizeof(void *) : sizeof(void *);
func_slot[opline->op2.constant] = opline->result.num;
}
break;
Expand Down
4 changes: 2 additions & 2 deletions Zend/Optimizer/zend_optimizer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ bool zend_optimizer_update_op2_const(zend_op_array *op_array,
drop_leading_backslash(val);
opline->op2.constant = zend_optimizer_add_literal(op_array, val);
zend_optimizer_add_literal_string(op_array, zend_string_tolower(Z_STR_P(val)));
opline->result.num = alloc_cache_slots(op_array, 1);
opline->result.num = alloc_cache_slots(op_array, 2);
break;
case ZEND_ASSIGN_STATIC_PROP:
case ZEND_ASSIGN_STATIC_PROP_REF:
Expand Down Expand Up @@ -422,7 +422,7 @@ bool zend_optimizer_update_op2_const(zend_op_array *op_array,
drop_leading_backslash(val);
opline->op2.constant = zend_optimizer_add_literal(op_array, val);
zend_optimizer_add_literal_string(op_array, zend_string_tolower(Z_STR_P(val)));
opline->result.num = alloc_cache_slots(op_array, 1);
opline->result.num = alloc_cache_slots(op_array, 2);
} else {
opline->op2.constant = zend_optimizer_add_literal(op_array, val);
}
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3706,7 +3706,7 @@ static void zend_compile_dynamic_call(znode *result, znode *name_node, zend_ast
opline->opcode = ZEND_INIT_FCALL_BY_NAME;
opline->op2_type = IS_CONST;
opline->op2.constant = zend_add_func_name_literal(str);
opline->result.num = zend_alloc_cache_slot();
opline->result.num = zend_alloc_cache_slots(2);
}
} else {
zend_emit_op(NULL, ZEND_INIT_DYNAMIC_CALL, NULL, name_node);
Expand Down
11 changes: 8 additions & 3 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3749,7 +3749,8 @@ ZEND_VM_HOT_HANDLER(59, ZEND_INIT_FCALL_BY_NAME, ANY, CONST, NUM|CACHE_SLOT)
zval *function_name, *func;
zend_execute_data *call;

fbc = CACHED_PTR(opline->result.num);
int num = opline->result.num;
fbc = CACHED_PTR(num);
if (UNEXPECTED(fbc == NULL)) {
function_name = (zval*)RT_CONSTANT(opline, opline->op2);
func = zend_hash_find_known_hash(EG(function_table), Z_STR_P(function_name+1));
Expand All @@ -3760,9 +3761,13 @@ ZEND_VM_HOT_HANDLER(59, ZEND_INIT_FCALL_BY_NAME, ANY, CONST, NUM|CACHE_SLOT)
if (EXPECTED(fbc->type == ZEND_USER_FUNCTION) && UNEXPECTED(!RUN_TIME_CACHE(&fbc->op_array))) {
init_func_run_time_cache(&fbc->op_array);
}
CACHE_PTR(opline->result.num, fbc);
CACHE_PTR(num, fbc);
CACHE_PTR(num + sizeof(void*), (void*)(uintptr_t)zend_vm_calc_used_stack(opline->extended_value, fbc));
}
call = _zend_vm_stack_push_call_frame(ZEND_CALL_NESTED_FUNCTION,
/* Because zend_vm_calc_used_stack depends only on the number of arguments and the function definition fbc,
* it can be cached in the run time cache. */
call = _zend_vm_stack_push_call_frame_ex(
(uint32_t)(uintptr_t)CACHED_PTR(num + sizeof(void*)), ZEND_CALL_NESTED_FUNCTION,
fbc, opline->extended_value, NULL);
call->prev_execute_data = EX(call);
EX(call) = call;
Expand Down
11 changes: 8 additions & 3 deletions Zend/zend_vm_execute.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3574,7 +3574,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_FCALL_BY_NAME
zval *function_name, *func;
zend_execute_data *call;

fbc = CACHED_PTR(opline->result.num);
int num = opline->result.num;
fbc = CACHED_PTR(num);
if (UNEXPECTED(fbc == NULL)) {
function_name = (zval*)RT_CONSTANT(opline, opline->op2);
func = zend_hash_find_known_hash(EG(function_table), Z_STR_P(function_name+1));
Expand All @@ -3585,9 +3586,13 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_FCALL_BY_NAME
if (EXPECTED(fbc->type == ZEND_USER_FUNCTION) && UNEXPECTED(!RUN_TIME_CACHE(&fbc->op_array))) {
init_func_run_time_cache(&fbc->op_array);
}
CACHE_PTR(opline->result.num, fbc);
CACHE_PTR(num, fbc);
CACHE_PTR(num + sizeof(void*), (void*)(uintptr_t)zend_vm_calc_used_stack(opline->extended_value, fbc));
}
call = _zend_vm_stack_push_call_frame(ZEND_CALL_NESTED_FUNCTION,
/* Because zend_vm_calc_used_stack depends only on the number of arguments and the function definition fbc,
* it can be cached in the run time cache. */
call = _zend_vm_stack_push_call_frame_ex(
(uint32_t)(uintptr_t)CACHED_PTR(num + sizeof(void*)), ZEND_CALL_NESTED_FUNCTION,
fbc, opline->extended_value, NULL);
call->prev_execute_data = EX(call);
EX(call) = call;
Expand Down
4 changes: 3 additions & 1 deletion ext/opcache/jit/zend_jit_arm64.dasc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8765,9 +8765,11 @@ static int zend_jit_init_fcall(dasm_State **Dst, const zend_op *opline, uint32_t
| ADD_SUB_64_WITH_CONST_32 add, FCARG2x, REG2, opline->result.num, TMP1
| EXT_CALL zend_jit_find_func_helper, REG0
} else if (opline->opcode == ZEND_INIT_FCALL_BY_NAME) {
/* TODO: Actually use the precomputed stack size in the JIT For ZEND_INIT_FCALL_BY_NAME */
| LOAD_ADDR FCARG1x, Z_STR_P(zv + 1);
| ADD_SUB_64_WITH_CONST_32 add, FCARG2x, REG2, opline->result.num, TMP1
| EXT_CALL zend_jit_find_func_helper, REG0
| LOAD_ADDR CARG3, opline
| EXT_CALL zend_jit_find_func_by_name_helper, REG0
} else if (opline->opcode == ZEND_INIT_NS_FCALL_BY_NAME) {
| LOAD_ADDR FCARG1x, zv;
| ADD_SUB_64_WITH_CONST_32 add, FCARG2x, REG2, opline->result.num, TMP1
Expand Down
18 changes: 18 additions & 0 deletions ext/opcache/jit/zend_jit_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,24 @@ static zend_function* ZEND_FASTCALL zend_jit_find_func_helper(zend_string *name,
return fbc;
}

static zend_function* ZEND_FASTCALL zend_jit_find_func_by_name_helper(zend_string *name, void **cache_slot, zend_op *opline)
{
zval *func = zend_hash_find_known_hash(EG(function_table), name);
zend_function *fbc;

if (UNEXPECTED(func == NULL)) {
/* When opcache.preload is used, func can be null when compiling - ext/opcache/tests/jit/bug81256.phpt */
return NULL;
}
fbc = Z_FUNC_P(func);
if (EXPECTED(fbc->type == ZEND_USER_FUNCTION) && UNEXPECTED(!RUN_TIME_CACHE(&fbc->op_array))) {
fbc = _zend_jit_init_func_run_time_cache(&fbc->op_array);
}
cache_slot[0] = fbc;
cache_slot[1] = (void*)(uintptr_t)zend_vm_calc_used_stack(opline->extended_value, fbc);
return fbc;
}

static zend_function* ZEND_FASTCALL zend_jit_find_ns_func_helper(zval *func_name, void **cache_slot)
{
zval *func = zend_hash_find_known_hash(EG(function_table), Z_STR_P(func_name + 1));
Expand Down
11 changes: 10 additions & 1 deletion ext/opcache/jit/zend_jit_x86.dasc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9350,7 +9350,16 @@ static int zend_jit_init_fcall(dasm_State **Dst, const zend_op *opline, uint32_t
} else if (opline->opcode == ZEND_INIT_FCALL_BY_NAME) {
| LOAD_ADDR FCARG1a, Z_STR_P(zv + 1);
| lea FCARG2a, aword [r2 + opline->result.num]
| EXT_CALL zend_jit_find_func_helper, r0
|.if X64
| LOAD_ADDR CARG3, opline
|.else
| sub r4, 12
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about why it's 12 bytes in other calls I've seen with 3 arguments rather than something smaller for a 4-byte pointer (in the examples I used as reference), but even if it was too large it wouldn't cause bugs

| push opline
|.endif
| EXT_CALL zend_jit_find_func_by_name_helper, r0
|.if not(X64)
| add r4, 12
|.endif
} else if (opline->opcode == ZEND_INIT_NS_FCALL_BY_NAME) {
| LOAD_ADDR FCARG1a, zv;
| lea FCARG2a, aword [r2 + opline->result.num]
Expand Down