-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Optimize ZEND_COUNT opcodes on arrays in the jit #5584
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.
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
Conversation
4f45167
to
c730720
Compare
9b6d568
to
79cd6aa
Compare
I updated this now that https://wiki.php.net/rfc/restrict_globals_usage has passed (related to 2279388 ) |
if (!zend_jit_count(&dasm_state, opline, op_array, op1_info)) { | ||
goto jit_failure; | ||
} | ||
goto done; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need to add it to zend_jit_trace.c for it to be meaningfully useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented in zend_jit_trace.c based on the handler for ZEND_STRLEN
ext/opcache/jit/zend_jit_x86.dasc
Outdated
// Anything containing objects or resources might throw when freed due to the destructors. | ||
zend_bool gc = (op1_info & (MAY_BE_ARRAY_OF_OBJECT|MAY_BE_ARRAY_OF_ARRAY|MAY_BE_ARRAY_OF_RESOURCE|MAY_BE_ARRAY_OF_REF)) != 0; | ||
| ZVAL_PTR_DTOR op1_addr, op1_info, gc, 0, opline | ||
if (gc) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the common practice here is to determine this using zend_may_throw (passed in).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to re-add this, I was running into issues because nNumOfElements was 32-bit when working on the tracing jit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, could use another look
ext/opcache/jit/zend_jit_x86.dasc
Outdated
| mov r0, [FCARG1a + offsetof(HashTable, nNumOfElements)] | ||
| SET_ZVAL_LVAL res_addr, r0 | ||
| SET_ZVAL_TYPE_INFO res_addr, IS_LONG | ||
if (opline->op1_type & (IS_VAR|IS_TMP_VAR)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use FREE_OP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Avoid the overhead of a call and checking types when the argument is definitely an array. Avoid the overhead of gc when `__destruct` won't get called. This seemed cheap enough to check for in the jit. Because of https://wiki.php.net/rfc/restrict_globals_usage we can be sure in the ZEND_COUNT handler that the array count does not have to be recomputed in php 8.1. The below example took 0.854 seconds before the optimization, and 0.564 seconds after the optimization, giving the same result ```php <?php /** @jit */ function bench_count(int $n): int { $total = 0; $arr = []; for ($i = 0; $i < $n; $i++) { $arr[] = $i; $total += count($arr); } return $total; } function main() { $n = 1000; $iterations = 50000; $start = microtime(true); $result = 0; for ($i = 0; $i < $iterations; $i++) { $result += bench_count($n); } $elapsed = microtime(true) - $start; printf("Total for n=%d, iterations=%d = %d, elapsed=%.3f\n", $n, $iterations, $result, $elapsed); } main(); ``` Before ```asm mov $0x7feb8cf8a858, %r15 mov $ZEND_COUNT_SPEC_CV_UNUSED_HANDLER, %rax call *%rax ``` After ```asm mov 0x70(%r14), %rdi - Copy the count from the `zend_array*` pointer mov %rdi, (%rax) - Store the count in the destination's value mov $0x4, 0x8(%rax) - Store IS_LONG(4) in the destination's type ``` And add tracing jit support
cbe688d
to
9835408
Compare
Everything should be fine. Merge this. |
Avoid the overhead of a call and checking types
when the argument is definitely an array.
Avoid the overhead of gc when
__destruct
won't get called.This seemed cheap enough to check for in the jit.
EDIT: Because of https://wiki.php.net/rfc/restrict_globals_usage
we can be sure in the ZEND_COUNT handler that the array count does not have to
be recomputed in php 8.1.
The below example took 0.854 seconds before the optimization,
and 0.564 seconds after the optimization, giving the same result
Before
After (much faster without a function call)