-
Notifications
You must be signed in to change notification settings - Fork 0
Sprintf optimize #2
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
Zend/zend_compile.c
Outdated
zend_op *init_opline = CG(active_op_array)->opcodes + rope_init_lineno; | ||
zend_compile_rope_finalize(result, rope_elements, init_opline, opline); | ||
|
||
zval_ptr_dtor_str(&format_string.u.constant); |
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.
This misses the optimizations from zend_compile_encaps_list()
, which concat()
s adjacent string constants. I'm not sure how important this is for sprintf
.
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.
Fairly unimportant, I'd say: Using a placeholder to insert a constant string is not really the expected use case of sprintf()
.
FWIW: Opcache will automatically merge ropes that are completely constant:
<?php
var_dump(sprintf("%s: %s-%s", __FILE__, "foo", "bar"));
$ sapi/cli/php -d zend_extension=(pwd)/modules/opcache.so -d opcache.enable_cli=1 -d opcache.opt_debug_level=0x10000 test.php
$_main:
; (lines=9, args=0, vars=0, tmps=5)
; (before optimizer)
; php-src/test.php:1-4
; return [] RANGE[0..0]
0000 INIT_FCALL 1 96 string("var_dump")
0001 T1 = ROPE_INIT 5 string("php-src/test.php")
0002 T1 = ROPE_ADD 1 T1 string(": ")
0003 T1 = ROPE_ADD 2 T1 string("foo")
0004 T1 = ROPE_ADD 3 T1 string("-")
0005 T0 = ROPE_END 4 T1 string("bar")
0006 SEND_VAL T0 1
0007 DO_ICALL
0008 RETURN int(1)
LIVE RANGES:
1: 0001 - 0005 (rope)
string(25) "php-src/test.php: foo-bar"
$ sapi/cli/php -d zend_extension=(pwd)/modules/opcache.so -d opcache.enable_cli=1 -d opcache.opt_debug_level=0x20000 test.php
$_main:
; (lines=4, args=0, vars=0, tmps=0)
; (after optimizer)
; php-src/test.php:1-4
0000 INIT_FCALL 1 96 string("var_dump")
0001 SEND_VAL string("php-src/test.php: foo-bar") 1
0002 DO_ICALL
0003 RETURN int(1)
string(25) "php-src/test.php: foo-bar"
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.
Fair enough. This can always be added later. And if opcache takes care of it, it's likely fairly unimportant.
372b967
to
b6f1fb1
Compare
This just extracts the implementation as-is into a dedicated function to make it reusable in preparation of a future commit.
… a single constant string Without this Opcache will trigger a use-after-free in `zend_optimizer_compact_literals()`. Co-authored-by: Ilija Tovilo <[email protected]>
f052bde
to
37127c2
Compare
This optimization will compile `sprintf()` using only `%s` placeholders into a rope at compile time, effectively making those calls equivalent to the use of string interpolation, with the added benefit of supporting arbitrary expressions instead of just expressions starting with a `$`. For a synthetic test using: <?php $a = 'foo'; $b = 'bar'; for ($i = 0; $i < 100_000_000; $i++) { sprintf("%s-%s", $a, $b); } This optimization yields a 2.1× performance improvement: $ hyperfine 'sapi/cli/php -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php' \ '/tmp/unoptimized -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php' Benchmark 1: sapi/cli/php -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php Time (mean ± σ): 1.869 s ± 0.033 s [User: 1.865 s, System: 0.003 s] Range (min … max): 1.840 s … 1.945 s 10 runs Benchmark 2: /tmp/unoptimized -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php Time (mean ± σ): 4.011 s ± 0.034 s [User: 4.006 s, System: 0.005 s] Range (min … max): 3.964 s … 4.079 s 10 runs Summary sapi/cli/php -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php ran 2.15 ± 0.04 times faster than /tmp/unoptimized -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php This optimization comes with a small and probably insignificant behavioral change: If one of the values cannot be (cleanly) converted to a string, for example when attempting to insert an object that is not `Stringable`, the resulting Exception will naturally not show the `sprintf()` call in the resulting stack trace, because there is no call to `sprintf()`. Nevertheless it will correctly point out the line of the `sprintf()` call as the source of the Exception, pointing the user towards the correct location.
37127c2
to
9fc7a73
Compare
even without sanitizers, it is reproducible but with the following ``` <?php $g = gmp_init(256); var_dump(gmp_pow($g, PHP_INT_MAX)); ``` we get this ``` AddressSanitizer:DEADLYSIGNAL ================================================================= ==286922==ERROR: AddressSanitizer: FPE on unknown address 0x03e8000460ca (pc 0x7faf6c69de5c bp 0x400000000000004 sp 0x7ffe9843c740 T0) #0 0x7faf6c69de5c in __pthread_kill_implementation nptl/pthread_kill.c:44 #1 0x7faf6c649c81 in __GI_raise ../sysdeps/posix/raise.c:26 #2 0x7faf6db9386c in __gmp_exception (/lib/x86_64-linux-gnu/libgmp.so.10+0xd86c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #3 0x7faf6db938d3 in __gmp_overflow_in_mpz (/lib/x86_64-linux-gnu/libgmp.so.10+0xd8d3) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #4 0x7faf6dbac95c in __gmpz_realloc (/lib/x86_64-linux-gnu/libgmp.so.10+0x2695c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #5 0x7faf6dba9038 in __gmpz_n_pow_ui (/lib/x86_64-linux-gnu/libgmp.so.10+0x23038) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #6 0x5565ae1ccd9f in zif_gmp_pow /home/dcarlier/Contribs/php-src/ext/gmp/gmp.c:1286 php#7 0x5565aee96ea9 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:1312 php#8 0x5565af144320 in execute_ex /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:56075 php#9 0x5565af160f07 in zend_execute /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:60439 php#10 0x5565aed6fafe in zend_execute_scripts /home/dcarlier/Contribs/php-src/Zend/zend.c:1842 php#11 0x5565aeae70a8 in php_execute_script /home/dcarlier/Contribs/php-src/main/main.c:2578 php#12 0x5565af532f4e in do_cli /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:964 php#13 0x5565af535877 in main /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:1334 php#14 0x7faf6c633d67 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 php#15 0x7faf6c633e24 in __libc_start_main_impl ../csu/libc-start.c:360 php#16 0x5565adc04040 in _start (/home/dcarlier/Contribs/php-src/sapi/cli/php+0x2604040) (BuildId: 949049955bdf8b7197390b1978a1dfc3ef6fdf38) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: FPE nptl/pthread_kill.c:44 in __pthread_kill_implementation ==286922==ABORTING ```
even without sanitizers, it is reproducible but with the following ``` <?php $g = gmp_init(256); var_dump(gmp_pow($g, PHP_INT_MAX)); ``` we get this ``` AddressSanitizer:DEADLYSIGNAL ================================================================= ==286922==ERROR: AddressSanitizer: FPE on unknown address 0x03e8000460ca (pc 0x7faf6c69de5c bp 0x400000000000004 sp 0x7ffe9843c740 T0) #0 0x7faf6c69de5c in __pthread_kill_implementation nptl/pthread_kill.c:44 #1 0x7faf6c649c81 in __GI_raise ../sysdeps/posix/raise.c:26 #2 0x7faf6db9386c in __gmp_exception (/lib/x86_64-linux-gnu/libgmp.so.10+0xd86c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #3 0x7faf6db938d3 in __gmp_overflow_in_mpz (/lib/x86_64-linux-gnu/libgmp.so.10+0xd8d3) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #4 0x7faf6dbac95c in __gmpz_realloc (/lib/x86_64-linux-gnu/libgmp.so.10+0x2695c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #5 0x7faf6dba9038 in __gmpz_n_pow_ui (/lib/x86_64-linux-gnu/libgmp.so.10+0x23038) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #6 0x5565ae1ccd9f in zif_gmp_pow /home/dcarlier/Contribs/php-src/ext/gmp/gmp.c:1286 php#7 0x5565aee96ea9 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:1312 php#8 0x5565af144320 in execute_ex /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:56075 php#9 0x5565af160f07 in zend_execute /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:60439 php#10 0x5565aed6fafe in zend_execute_scripts /home/dcarlier/Contribs/php-src/Zend/zend.c:1842 php#11 0x5565aeae70a8 in php_execute_script /home/dcarlier/Contribs/php-src/main/main.c:2578 php#12 0x5565af532f4e in do_cli /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:964 php#13 0x5565af535877 in main /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:1334 php#14 0x7faf6c633d67 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 php#15 0x7faf6c633e24 in __libc_start_main_impl ../csu/libc-start.c:360 php#16 0x5565adc04040 in _start (/home/dcarlier/Contribs/php-src/sapi/cli/php+0x2604040) (BuildId: 949049955bdf8b7197390b1978a1dfc3ef6fdf38) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: FPE nptl/pthread_kill.c:44 in __pthread_kill_implementation ==286922==ABORTING ``` close phpGH-16384
``` ext/gd/libgd/gd.c:2275:14: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int' #0 0x5d6a2103e1db in php_gd_gdImageCopy /home/dcarlier/Contribs/php-src/ext/gd/libgd/gd.c:2275 #1 0x5d6a210a2b63 in gdImageCrop /home/dcarlier/Contribs/php-src/ext/gd/libgd/gd_crop.c:57 #2 0x5d6a21018ca4 in zif_imagecrop /home/dcarlier/Contribs/php-src/ext/gd/gd.c:3575 #3 0x5d6a21e46e7a in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:1337 #4 0x5d6a221188da in execute_ex /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:57246 #5 0x5d6a221366bd in zend_execute /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:61634 #6 0x5d6a21d107a6 in zend_execute_scripts /home/dcarlier/Contribs/php-src/Zend/zend.c:1895 php#7 0x5d6a21a63409 in php_execute_script /home/dcarlier/Contribs/php-src/main/main.c:2529 php#8 0x5d6a22516d5e in do_cli /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:966 php#9 0x5d6a2251981d in main /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:1341 php#10 0x7f10d002a3b7 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 php#11 0x7f10d002a47a in __libc_start_main_impl ../csu/libc-start.c:360 php#12 0x5d6a20a06da4 in _start (/home/dcarlier/Contribs/php-src/sapi/cli/php+0x2806da4) (BuildId: d9a79c7e0e4872311439d7313cb3a81fe04190a2) ``` close phpGH-18006
No description provided.