Repository navigation
ext/standard: Optimize var_export - #23189
kamil-tekiela wants to merge 6 commits into
Conversation
ArtUkrainskiy
left a comment
There was a problem hiding this comment.
var_export() of a string with enough NUL bytes overflows the escaped length on 32-bit builds and crashes, details and a replacement inline. Measurements in a separate comment.
|
I measured this on master c65321c (dc5f795 merges with a conflict in Change in CPU cycles per
Long strings get slower. Strings, arrays, config arrays and objects of small and medium size take 15–68% fewer cycles, since the temporary strings and The one-pass version in my review comment (a byte class table, plain runs appended with
Correctness: output identical to master with the PR and with that version, on all four compilers, under ASan/UBSan and on a 32-bit build, for every byte value as value and key, all strings up to 4 bytes over Minor: On Full tables, scripts and data: https://github.com/ArtUkrainskiy/php-src-bench/tree/main/reports/pr-23189-var-export |
cd6926f to
999132b
Compare
Based on smart_str_append_escaped
Co-authored-by: Artem Ukrainskiy <art.ukrainskiy@gmail.com>
999132b to
1316367
Compare
The first commit replaces
sprintfwithmemset.The second commit replaces
php_addcslashesandphp_str_to_strwith a custom function based on thesmart_str_append_escapedfunction from zend_smart_str.c. At one of the 3 call sites, thephp_str_to_strwas absent, and I didn't know what to do, which is why I added the flag. This makes the most sense to me, but I could also make it separate from the new function.I don't know whether
buffer_append_spacesshould haveinlineor not. I assume it shouldn't.In my simple round of testing, this proved to make
var_export3x faster.