Skip to content

Improve performance of instantiating exceptions/errors #18442

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,7 @@ PHP 8.5 UPGRADE NOTES
- Core:
. Remove OPcodes for identity comparisons against booleans, particularly
for the match(true) pattern.
. Creating exception objects is now much faster.

- ReflectionProperty:
. Improved performance of the following methods: getValue(), getRawValue(),
Expand Down
69 changes: 43 additions & 26 deletions Zend/zend_exceptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,33 @@ ZEND_API void zend_clear_exception(void) /* {{{ */
}
/* }}} */

/* Same as writing to OBJ_PROP_NUM() when there are no hooks,
* but checks the offset is correct when Zend is built in debug mode.
* This is faster than going through the regular property write routine when the offset is known at compile time. */
static void zend_update_property_num_checked(zend_object *object, uint32_t prop_num, zend_string *member, zval *value)
{
if (UNEXPECTED(object->ce->num_hooked_props > 0)) {
/* Property may have been overridden with a hook. */
zend_update_property_ex(object->ce, object, member, value);
zval_ptr_dtor(value);
return;
}
#if ZEND_DEBUG
zend_class_entry *old_scope = EG(fake_scope);
EG(fake_scope) = i_get_exception_base(object);
const zend_property_info *prop_info = zend_get_property_info(object->ce, member, true);
ZEND_ASSERT(OBJ_PROP_TO_NUM(prop_info->offset) == prop_num);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you can depend on this since #17870. These properties are not final, so a child class may add hooks to them. A simple solution would be to check for ce->num_hooked_props == 0 and use a fallback to zend_update_property_ex() otherwise.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine for the private properties though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, that would be fine. Not all properties are here though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, right, so that has likely also broken the same optimization I did in DOM for some props a long time ago. Since introducing hooks in a subclass of an exception type is likely uncommon, I'll just add the num_hooked_props check I guess and figure it out from there.

EG(fake_scope) = old_scope;
#endif
zval *zv = OBJ_PROP_NUM(object, prop_num);
zval_ptr_safe_dtor(zv);
ZVAL_COPY_VALUE(zv, value);
}

static zend_object *zend_default_exception_new(zend_class_entry *class_type) /* {{{ */
{
zval tmp;
zval trace;
zend_class_entry *base_ce;
zend_string *filename;

zend_object *object = zend_objects_new(class_type);
Expand All @@ -269,26 +291,23 @@ static zend_object *zend_default_exception_new(zend_class_entry *class_type) /*
0,
EG(exception_ignore_args) ? DEBUG_BACKTRACE_IGNORE_ARGS : 0, 0);
} else {
array_init(&trace);
ZVAL_EMPTY_ARRAY(&trace);
}
Z_SET_REFCOUNT(trace, 0);

base_ce = i_get_exception_base(object);
zend_update_property_num_checked(object, 5, ZSTR_KNOWN(ZEND_STR_TRACE), &trace);

if (EXPECTED((class_type != zend_ce_parse_error && class_type != zend_ce_compile_error)
|| !(filename = zend_get_compiled_filename()))) {
ZVAL_STRING(&tmp, zend_get_executed_filename());
zend_update_property_ex(base_ce, object, ZSTR_KNOWN(ZEND_STR_FILE), &tmp);
zval_ptr_dtor(&tmp);
zend_update_property_num_checked(object, 3, ZSTR_KNOWN(ZEND_STR_FILE), &tmp);
ZVAL_LONG(&tmp, zend_get_executed_lineno());
zend_update_property_ex(base_ce, object, ZSTR_KNOWN(ZEND_STR_LINE), &tmp);
zend_update_property_num_checked(object, 4, ZSTR_KNOWN(ZEND_STR_LINE), &tmp);
} else {
ZVAL_STR(&tmp, filename);
zend_update_property_ex(base_ce, object, ZSTR_KNOWN(ZEND_STR_FILE), &tmp);
ZVAL_STR_COPY(&tmp, filename);
zend_update_property_num_checked(object, 3, ZSTR_KNOWN(ZEND_STR_FILE), &tmp);
ZVAL_LONG(&tmp, zend_get_compiled_lineno());
zend_update_property_ex(base_ce, object, ZSTR_KNOWN(ZEND_STR_LINE), &tmp);
zend_update_property_num_checked(object, 4, ZSTR_KNOWN(ZEND_STR_LINE), &tmp);
}
zend_update_property_ex(base_ce, object, ZSTR_KNOWN(ZEND_STR_TRACE), &trace);

return object;
}
Expand All @@ -308,27 +327,26 @@ ZEND_METHOD(Exception, __construct)
zend_string *message = NULL;
zend_long code = 0;
zval tmp, *object, *previous = NULL;
zend_class_entry *base_ce;

object = ZEND_THIS;
base_ce = i_get_exception_base(Z_OBJ_P(object));

if (zend_parse_parameters(ZEND_NUM_ARGS(), "|SlO!", &message, &code, &previous, zend_ce_throwable) == FAILURE) {
RETURN_THROWS();
}

if (message) {
ZVAL_STR(&tmp, message);
zend_update_property_ex(base_ce, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_MESSAGE), &tmp);
ZVAL_STR_COPY(&tmp, message);
zend_update_property_num_checked(Z_OBJ_P(object), 0, ZSTR_KNOWN(ZEND_STR_MESSAGE), &tmp);
}

if (code) {
ZVAL_LONG(&tmp, code);
zend_update_property_ex(base_ce, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_CODE), &tmp);
zend_update_property_num_checked(Z_OBJ_P(object), 2, ZSTR_KNOWN(ZEND_STR_CODE), &tmp);
}

if (previous) {
zend_update_property_ex(base_ce, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_PREVIOUS), previous);
Z_ADDREF_P(previous);
zend_update_property_num_checked(Z_OBJ_P(object), 6, ZSTR_KNOWN(ZEND_STR_PREVIOUS), previous);
}
}
/* }}} */
Expand Down Expand Up @@ -368,34 +386,33 @@ ZEND_METHOD(ErrorException, __construct)

if (message) {
ZVAL_STR_COPY(&tmp, message);
zend_update_property_ex(zend_ce_exception, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_MESSAGE), &tmp);
zval_ptr_dtor(&tmp);
zend_update_property_num_checked(Z_OBJ_P(object), 0, ZSTR_KNOWN(ZEND_STR_MESSAGE), &tmp);
}

if (code) {
ZVAL_LONG(&tmp, code);
zend_update_property_ex(zend_ce_exception, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_CODE), &tmp);
zend_update_property_num_checked(Z_OBJ_P(object), 2, ZSTR_KNOWN(ZEND_STR_CODE), &tmp);
}

if (previous) {
zend_update_property_ex(zend_ce_exception, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_PREVIOUS), previous);
Z_ADDREF_P(previous);
zend_update_property_num_checked(Z_OBJ_P(object), 6, ZSTR_KNOWN(ZEND_STR_PREVIOUS), previous);
}

ZVAL_LONG(&tmp, severity);
zend_update_property_ex(zend_ce_exception, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_SEVERITY), &tmp);
zend_update_property_num_checked(Z_OBJ_P(object), 7, ZSTR_KNOWN(ZEND_STR_SEVERITY), &tmp);

if (filename) {
ZVAL_STR_COPY(&tmp, filename);
zend_update_property_ex(zend_ce_exception, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_FILE), &tmp);
zval_ptr_dtor(&tmp);
zend_update_property_num_checked(Z_OBJ_P(object), 3, ZSTR_KNOWN(ZEND_STR_FILE), &tmp);
}

if (!lineno_is_null) {
ZVAL_LONG(&tmp, lineno);
zend_update_property_ex(zend_ce_exception, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_LINE), &tmp);
zend_update_property_num_checked(Z_OBJ_P(object), 4, ZSTR_KNOWN(ZEND_STR_LINE), &tmp);
} else if (filename) {
ZVAL_LONG(&tmp, 0);
zend_update_property_ex(zend_ce_exception, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_LINE), &tmp);
zend_update_property_num_checked(Z_OBJ_P(object), 4, ZSTR_KNOWN(ZEND_STR_LINE), &tmp);
}
}
/* }}} */
Expand Down