From 1463f2ba96da83f721aa64ad03e28d4613d85f3e Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Tue, 4 Feb 2025 17:39:05 +0100 Subject: [PATCH 1/3] Optimize ReflectionProperty::getValue(), ::getRawValue(), ::isInitialized(), ::setValue(), ::setRawValue() - Pass a cache slot to the property handler - Inline the simple case of fetching a declared property - Use the new parameter parsing API This makes these methods 12% to 60% faster. Using the new parameter parsing API had the most impact, by far. --- ext/reflection/php_reflection.c | 119 +++++++++++++++++++++++--------- 1 file changed, 87 insertions(+), 32 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index fdde5998db93f..015411d2be873 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -132,6 +132,7 @@ PHPAPI zend_class_entry *reflection_property_hook_type_ptr; typedef struct _property_reference { zend_property_info *prop; zend_string *unmangled_name; + void *cache_slot[3]; } property_reference; /* Struct for parameters */ @@ -1506,6 +1507,7 @@ static void reflection_property_factory(zend_class_entry *ce, zend_string *name, reference = (property_reference*) emalloc(sizeof(property_reference)); reference->prop = prop; reference->unmangled_name = zend_string_copy(name); + memset(reference->cache_slot, 0, sizeof(reference->cache_slot)); intern->ptr = reference; intern->ref_type = REF_TYPE_PROPERTY; intern->ce = ce; @@ -5643,6 +5645,7 @@ ZEND_METHOD(ReflectionProperty, __construct) reference = (property_reference*) emalloc(sizeof(property_reference)); reference->prop = dynam_prop ? NULL : property_info; reference->unmangled_name = zend_string_copy(name); + memset(reference->cache_slot, 0, sizeof(reference->cache_slot)); intern->ptr = reference; intern->ref_type = REF_TYPE_PROPERTY; intern->ce = ce; @@ -5794,9 +5797,10 @@ ZEND_METHOD(ReflectionProperty, getValue) zval *object = NULL; zval *member_p = NULL; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "|o!", &object) == FAILURE) { - RETURN_THROWS(); - } + ZEND_PARSE_PARAMETERS_START(0, 1) + Z_PARAM_OPTIONAL + Z_PARAM_OBJECT_EX(object, 1, 0) + ZEND_PARSE_PARAMETERS_END(); GET_REFLECTION_OBJECT_PTR(ref); @@ -5813,13 +5817,29 @@ ZEND_METHOD(ReflectionProperty, getValue) RETURN_THROWS(); } - /* TODO: Should this always use intern->ce? */ - if (!instanceof_function(Z_OBJCE_P(object), ref->prop ? ref->prop->ce : intern->ce)) { - _DO_THROW("Given object is not an instance of the class this property was declared in"); - RETURN_THROWS(); + if (ref->cache_slot[0] == Z_OBJCE_P(object)) { + uintptr_t prop_offset = (uintptr_t) ref->cache_slot[1]; + + if (EXPECTED(IS_VALID_PROPERTY_OFFSET(prop_offset))) { + zval *retval = OBJ_PROP(Z_OBJ_P(object), prop_offset); + if (EXPECTED(Z_TYPE_INFO_P(retval) != IS_UNDEF)) { + RETURN_COPY_DEREF(retval); + } + } + } else { + /* TODO: Should this always use intern->ce? */ + if (!instanceof_function(Z_OBJCE_P(object), ref->prop ? ref->prop->ce : intern->ce)) { + _DO_THROW("Given object is not an instance of the class this property was declared in"); + RETURN_THROWS(); + } } - member_p = zend_read_property_ex(intern->ce, Z_OBJ_P(object), ref->unmangled_name, 0, &rv); + zend_class_entry *old_scope = EG(fake_scope); + EG(fake_scope) = intern->ce; + member_p = Z_OBJ_P(object)->handlers->read_property(Z_OBJ_P(object), + ref->unmangled_name, BP_VAR_R, ref->cache_slot, &rv); + EG(fake_scope) = old_scope; + if (member_p != &rv) { RETURN_COPY_DEREF(member_p); } else { @@ -5873,7 +5893,10 @@ ZEND_METHOD(ReflectionProperty, setValue) Z_PARAM_ZVAL(value) ZEND_PARSE_PARAMETERS_END(); - zend_update_property_ex(intern->ce, object, ref->unmangled_name, value); + zend_class_entry *old_scope = EG(fake_scope); + EG(fake_scope) = intern->ce; + object->handlers->write_property(object, ref->unmangled_name, value, ref->cache_slot); + EG(fake_scope) = old_scope; } } /* }}} */ @@ -5897,15 +5920,26 @@ ZEND_METHOD(ReflectionProperty, getRawValue) property_reference *ref; zval *object; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "o", &object) == FAILURE) { - RETURN_THROWS(); - } + ZEND_PARSE_PARAMETERS_START(1, 1) + Z_PARAM_OBJECT(object) + ZEND_PARSE_PARAMETERS_END(); GET_REFLECTION_OBJECT_PTR(ref); - if (!instanceof_function(Z_OBJCE_P(object), intern->ce)) { - _DO_THROW("Given object is not an instance of the class this property was declared in"); - RETURN_THROWS(); + if (ref->cache_slot[0] == Z_OBJCE_P(object)) { + uintptr_t prop_offset = (uintptr_t) ref->cache_slot[1]; + + if (EXPECTED(IS_VALID_PROPERTY_OFFSET(prop_offset))) { + zval *retval = OBJ_PROP(Z_OBJ_P(object), prop_offset); + if (EXPECTED(Z_TYPE_INFO_P(retval) != IS_UNDEF)) { + RETURN_COPY_DEREF(retval); + } + } + } else { + if (!instanceof_function(Z_OBJCE_P(object), intern->ce)) { + _DO_THROW("Given object is not an instance of the class this property was declared in"); + RETURN_THROWS(); + } } zend_property_info *prop = reflection_property_get_effective_prop(ref, @@ -5918,7 +5952,12 @@ ZEND_METHOD(ReflectionProperty, getRawValue) if (!prop || !prop->hooks || !prop->hooks[ZEND_PROPERTY_HOOK_GET]) { zval rv; - zval *member_p = zend_read_property_ex(intern->ce, Z_OBJ_P(object), ref->unmangled_name, 0, &rv); + zend_class_entry *old_scope = EG(fake_scope); + EG(fake_scope) = intern->ce; + zval *member_p = Z_OBJ_P(object)->handlers->read_property( + Z_OBJ_P(object), ref->unmangled_name, BP_VAR_R, + ref->cache_slot, &rv); + EG(fake_scope) = old_scope; if (member_p != &rv) { RETURN_COPY_DEREF(member_p); @@ -5935,11 +5974,14 @@ ZEND_METHOD(ReflectionProperty, getRawValue) } static void reflection_property_set_raw_value(zend_property_info *prop, - zend_string *unmangled_name, reflection_object *intern, + zend_string *unmangled_name, void *cache_slot[3], reflection_object *intern, zend_object *object, zval *value) { if (!prop || !prop->hooks || !prop->hooks[ZEND_PROPERTY_HOOK_SET]) { - zend_update_property_ex(intern->ce, object, unmangled_name, value); + zend_class_entry *old_scope = EG(fake_scope); + EG(fake_scope) = intern->ce; + object->handlers->write_property(object, unmangled_name, value, cache_slot); + EG(fake_scope) = old_scope; } else { zend_function *func = zend_get_property_hook_trampoline(prop, ZEND_PROPERTY_HOOK_SET, unmangled_name); zend_call_known_instance_method_with_1_params(func, object, NULL, value); @@ -5955,9 +5997,10 @@ ZEND_METHOD(ReflectionProperty, setRawValue) GET_REFLECTION_OBJECT_PTR(ref); - if (zend_parse_parameters(ZEND_NUM_ARGS(), "oz", &object, &value) == FAILURE) { - RETURN_THROWS(); - } + ZEND_PARSE_PARAMETERS_START(2, 2) { + Z_PARAM_OBJECT(object) + Z_PARAM_ZVAL(value) + } ZEND_PARSE_PARAMETERS_END(); zend_property_info *prop = reflection_property_get_effective_prop(ref, intern->ce, Z_OBJ_P(object)); @@ -5967,7 +6010,8 @@ ZEND_METHOD(ReflectionProperty, setRawValue) RETURN_THROWS(); } - reflection_property_set_raw_value(prop, ref->unmangled_name, intern, Z_OBJ_P(object), value); + reflection_property_set_raw_value(prop, ref->unmangled_name, + ref->cache_slot, intern, Z_OBJ_P(object), value); } static zend_result reflection_property_check_lazy_compatible( @@ -6046,8 +6090,8 @@ ZEND_METHOD(ReflectionProperty, setRawValueWithoutLazyInitialization) /* Do not trigger initialization */ Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_LAZY; - reflection_property_set_raw_value(prop, ref->unmangled_name, intern, object, - value); + reflection_property_set_raw_value(prop, ref->unmangled_name, + ref->cache_slot, intern, object, value); /* Mark property as lazy again if an exception prevented update */ if (EG(exception) && prop_was_lazy && Z_TYPE_P(var_ptr) == IS_UNDEF @@ -6143,9 +6187,10 @@ ZEND_METHOD(ReflectionProperty, isInitialized) zval *object = NULL; zval *member_p = NULL; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "|o!", &object) == FAILURE) { - RETURN_THROWS(); - } + ZEND_PARSE_PARAMETERS_START(0, 1) + Z_PARAM_OPTIONAL + Z_PARAM_OBJECT_EX(object, 1, 0) + ZEND_PARSE_PARAMETERS_END(); GET_REFLECTION_OBJECT_PTR(ref); @@ -6164,15 +6209,25 @@ ZEND_METHOD(ReflectionProperty, isInitialized) RETURN_THROWS(); } - /* TODO: Should this always use intern->ce? */ - if (!instanceof_function(Z_OBJCE_P(object), ref->prop ? ref->prop->ce : intern->ce)) { - _DO_THROW("Given object is not an instance of the class this property was declared in"); - RETURN_THROWS(); + if (ref->cache_slot[0] == Z_OBJCE_P(object)) { + uintptr_t prop_offset = (uintptr_t) ref->cache_slot[1]; + + if (EXPECTED(IS_VALID_PROPERTY_OFFSET(prop_offset))) { + zval *value = OBJ_PROP(Z_OBJ_P(object), prop_offset); + RETURN_BOOL(Z_TYPE_INFO_P(value) != IS_UNDEF); + } + } else { + /* TODO: Should this always use intern->ce? */ + if (!instanceof_function(Z_OBJCE_P(object), ref->prop ? ref->prop->ce : intern->ce)) { + _DO_THROW("Given object is not an instance of the class this property was declared in"); + RETURN_THROWS(); + } } old_scope = EG(fake_scope); EG(fake_scope) = intern->ce; - retval = Z_OBJ_HT_P(object)->has_property(Z_OBJ_P(object), ref->unmangled_name, ZEND_PROPERTY_EXISTS, NULL); + retval = Z_OBJ_HT_P(object)->has_property(Z_OBJ_P(object), + ref->unmangled_name, ZEND_PROPERTY_EXISTS, ref->cache_slot); EG(fake_scope) = old_scope; RETVAL_BOOL(retval); From a9d99e5e705c6e595379eebef3e3aded9c84cb7e Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Tue, 11 Feb 2025 12:12:14 +0100 Subject: [PATCH 2/3] Move the instanceof check before the cached path --- ext/reflection/php_reflection.c | 34 ++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 015411d2be873..efa9f8e86e915 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -5817,6 +5817,12 @@ ZEND_METHOD(ReflectionProperty, getValue) RETURN_THROWS(); } + /* TODO: Should this always use intern->ce? */ + if (!instanceof_function(Z_OBJCE_P(object), ref->prop ? ref->prop->ce : intern->ce)) { + _DO_THROW("Given object is not an instance of the class this property was declared in"); + RETURN_THROWS(); + } + if (ref->cache_slot[0] == Z_OBJCE_P(object)) { uintptr_t prop_offset = (uintptr_t) ref->cache_slot[1]; @@ -5826,12 +5832,6 @@ ZEND_METHOD(ReflectionProperty, getValue) RETURN_COPY_DEREF(retval); } } - } else { - /* TODO: Should this always use intern->ce? */ - if (!instanceof_function(Z_OBJCE_P(object), ref->prop ? ref->prop->ce : intern->ce)) { - _DO_THROW("Given object is not an instance of the class this property was declared in"); - RETURN_THROWS(); - } } zend_class_entry *old_scope = EG(fake_scope); @@ -5926,6 +5926,11 @@ ZEND_METHOD(ReflectionProperty, getRawValue) GET_REFLECTION_OBJECT_PTR(ref); + if (!instanceof_function(Z_OBJCE_P(object), intern->ce)) { + _DO_THROW("Given object is not an instance of the class this property was declared in"); + RETURN_THROWS(); + } + if (ref->cache_slot[0] == Z_OBJCE_P(object)) { uintptr_t prop_offset = (uintptr_t) ref->cache_slot[1]; @@ -5935,11 +5940,6 @@ ZEND_METHOD(ReflectionProperty, getRawValue) RETURN_COPY_DEREF(retval); } } - } else { - if (!instanceof_function(Z_OBJCE_P(object), intern->ce)) { - _DO_THROW("Given object is not an instance of the class this property was declared in"); - RETURN_THROWS(); - } } zend_property_info *prop = reflection_property_get_effective_prop(ref, @@ -6209,6 +6209,12 @@ ZEND_METHOD(ReflectionProperty, isInitialized) RETURN_THROWS(); } + /* TODO: Should this always use intern->ce? */ + if (!instanceof_function(Z_OBJCE_P(object), ref->prop ? ref->prop->ce : intern->ce)) { + _DO_THROW("Given object is not an instance of the class this property was declared in"); + RETURN_THROWS(); + } + if (ref->cache_slot[0] == Z_OBJCE_P(object)) { uintptr_t prop_offset = (uintptr_t) ref->cache_slot[1]; @@ -6216,12 +6222,6 @@ ZEND_METHOD(ReflectionProperty, isInitialized) zval *value = OBJ_PROP(Z_OBJ_P(object), prop_offset); RETURN_BOOL(Z_TYPE_INFO_P(value) != IS_UNDEF); } - } else { - /* TODO: Should this always use intern->ce? */ - if (!instanceof_function(Z_OBJCE_P(object), ref->prop ? ref->prop->ce : intern->ce)) { - _DO_THROW("Given object is not an instance of the class this property was declared in"); - RETURN_THROWS(); - } } old_scope = EG(fake_scope); From e60bf0975144c1abac2ef2698d147d226cf43ce0 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Mon, 17 Feb 2025 11:52:40 +0100 Subject: [PATCH 3/3] Use Z_ISUNDEF_P --- ext/reflection/php_reflection.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index efa9f8e86e915..db739f33f3050 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -5828,7 +5828,7 @@ ZEND_METHOD(ReflectionProperty, getValue) if (EXPECTED(IS_VALID_PROPERTY_OFFSET(prop_offset))) { zval *retval = OBJ_PROP(Z_OBJ_P(object), prop_offset); - if (EXPECTED(Z_TYPE_INFO_P(retval) != IS_UNDEF)) { + if (EXPECTED(!Z_ISUNDEF_P(retval))) { RETURN_COPY_DEREF(retval); } } @@ -5936,7 +5936,7 @@ ZEND_METHOD(ReflectionProperty, getRawValue) if (EXPECTED(IS_VALID_PROPERTY_OFFSET(prop_offset))) { zval *retval = OBJ_PROP(Z_OBJ_P(object), prop_offset); - if (EXPECTED(Z_TYPE_INFO_P(retval) != IS_UNDEF)) { + if (EXPECTED(!Z_ISUNDEF_P(retval))) { RETURN_COPY_DEREF(retval); } } @@ -6220,7 +6220,7 @@ ZEND_METHOD(ReflectionProperty, isInitialized) if (EXPECTED(IS_VALID_PROPERTY_OFFSET(prop_offset))) { zval *value = OBJ_PROP(Z_OBJ_P(object), prop_offset); - RETURN_BOOL(Z_TYPE_INFO_P(value) != IS_UNDEF); + RETURN_BOOL(!Z_ISUNDEF_P(value)); } }