Expose zend_reflection_property_set_raw_value, zend_reflection_property_set_raw_value_without_lazy_initialization#21763
Conversation
…ty_set_raw_value_without_lazy_initialization
…lable
PHP 8.6 exposes zend_reflection_property_set_raw_value{,_without_lazy_initialization}
as PHPAPI (php/php-src#21763), moving the lazy-prop
bookkeeping into ext/reflection. Use the PHPAPI direct on 8.6+ and keep the
userland ReflectionProperty round-trip as a fallback for 8.4 / 8.5.
Depends on php/php-src#21763 landing; holding here until the patch is merged
and the target PHP version is known.
…lable
PHP 8.6 exposes zend_reflection_property_set_raw_value{,_without_lazy_initialization}
as PHPAPI (php/php-src#21763), moving the lazy-prop
bookkeeping into ext/reflection. Use the PHPAPI direct on 8.6+ and keep the
userland ReflectionProperty round-trip as a fallback for 8.4 / 8.5.
Depends on php/php-src#21763 landing; holding here until the patch is merged
and the target PHP version is known.
|
Not sure the current interface is the best to expose, e.g. the What is the issue with having other extensions just call the reflection methods via the normal mechanism for calling class methods? |
CPU overhead! |
|
@DanielEScherzer I've documented the If you prefer, I can change the API to something like this: typedef reflection_object zend_reflection_property;
PHPAPI zend_reflection_property *zend_reflection_property_create(zend_class_entry *ce, zend_string *name);
PHPAPI void zend_reflection_property_set_raw_value(zend_reflection_property *refl_prop, zend_object *object, zval *value);
In addition to being faster, this also provides a nicer API. Instantiating objects and calling methods is not straightforward in C. |
…lable
PHP 8.6 exposes zend_reflection_property_set_raw_value{,_without_lazy_initialization}
as PHPAPI (php/php-src#21763), moving the lazy-prop
bookkeeping into ext/reflection. Use the PHPAPI direct on 8.6+ and keep the
userland ReflectionProperty round-trip as a fallback for 8.4 / 8.5.
Depends on php/php-src#21763 landing; holding here until the patch is merged
and the target PHP version is known.
|
The other thing to mention is calling a "PHP" function/method from C AFAIK requires setting up new VM stacks compared to "just" running in a loop. The other thing is that for global functions those can be disabled by an INI setting. (For classes and class methods that's no longer an issue since 8.5 as the disable_class INI setting got removed) |
| * cache_slot can be used again with the same 'unmangled_name' and 'scope'. | ||
| * Must be zeroed on first use. May be NULL. */ | ||
| PHPAPI void zend_reflection_property_set_raw_value( | ||
| const zend_property_info *prop, zend_string *unmangled_name, |
There was a problem hiding this comment.
can we add a bit more documentation? I assume
zend_object *objectis what object we are setting a property onzend_property_info *propis the property to setzval *valueis the value to set
What is the unmangled_name here - if it is the same as the property name, why is a separate parameter needed? The unmangled name could already be retrieved from zend_get_unmangled_property_name(prop-> name)
What does the scope represent - since this is reflection, it doesn't matter what the calling scope is, visibility gets bypassed. Is it the class of the object? Then is it already available as object->ce, so why is a separate parameter needed
Internally reflection might pass the unmangled name and the class entry separately because they are already available and for performance, but that shouldn't be required from external callers
| GET_REFLECTION_OBJECT_PTR(ref); | ||
|
|
||
| zend_property_info *prop = ref->prop; | ||
| const zend_property_info *prop = ref->prop; |
There was a problem hiding this comment.
if you want to split up the marking of things as const into a different patch that one should be easier to review and merge while this is still being discussed
This exposes the functionality of
ReflectionProperty::setRawValue()andReflectionProperty::setRawValueWithoutLazyInitialiation()to C extensions.Test failure unrelated.