Generated arginfo header files: use known strings for prop names when… #15751
Conversation
DanielEScherzer
requested review from
nielsdos,
bukka,
SakiTakamachi,
adoy,
Girgias and
kocsismate
as code owners
github-actions
bot
added
Category: Build System
Category: Engine
Extension: bcmath
Extension: curl
Extension: dom
Extension: libxml
Extension: mysqli
Extension: pdo (core)
Extension: reflection
Extension: soap
Extension: standard
Extension: tidy
Extension: tokenizer
Extension: xmlreader
labels
… possible Instead of allocating, using, and then releasing a zend_string for every property name unconditionally, only do so when the minimum supported version of PHP does not have that string in its known strings (ZEND_KNOWN_STRINGS). If the string is already known, just use the known version directly. This is already done for some non-generated class registrations, e.g. in `zend_enum_register_props()`.
DanielEScherzer
force-pushed
the
known-strings
branch
from
ac1684e to
0a146f0
Compare
@kocsismate would you mind taking another look? No objections from other reviewers. |
Sorry for the delay... But I've rechecked it. I'll approve it once the comments are addressed. |
Drop spaces from brackets, don't hard code latest version
No worries, comments have been addressed |
|
@kocsismate I see that you approved this patch - would you mind also merging it? I don't have the permissions to do so myself |
|
do we expect a measurable memory optimization from this PR? If so, any approximation? |
I was wondering about the same question first (#15751 (comment)), and my estimation is that it has negligible improvements. |
php#15751) Instead of allocating, using, and then releasing a zend_string for every property name unconditionally, only do so when the minimum supported version of PHP does not have that string in its known strings (ZEND_KNOWN_STRINGS). If the string is already known, just use the known version directly. This is already done for some non-generated class registrations, e.g. in `zend_enum_register_props()`.
I forgot a `$this->` in php#15751
I forgot a `$this->` in #15751
… possible
Instead of allocating, using, and then releasing a zend_string for every property name unconditionally, only do so when the minimum supported version of PHP does not have that string in its known strings (ZEND_KNOWN_STRINGS). If the string is already known, just use the known version directly. This is already done for some non-generated class registrations, e.g. in
zend_enum_register_props().