-
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.