Skip to content

Reduce memory overhead of DatePeriod via virtual properties #15598

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

Merged
merged 7 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Reduce memory overhead of DatePeriod via virtual properties
Related to #11644 and #13988
  • Loading branch information
kocsismate committed Sep 26, 2024
commit f6e03a8c61109f5fffec27c8bff91c5cb6398f61
105 changes: 58 additions & 47 deletions ext/date/php_date.c
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ static zval *date_interval_get_property_ptr_ptr(zend_object *object, zend_string
static zval *date_period_read_property(zend_object *object, zend_string *name, int type, void **cache_slot, zval *rv);
static zval *date_period_write_property(zend_object *object, zend_string *name, zval *value, void **cache_slot);
static zval *date_period_get_property_ptr_ptr(zend_object *object, zend_string *name, int type, void **cache_slot);

static HashTable *date_period_get_properties_for(zend_object *object, zend_prop_purpose purpose);
static int date_object_compare_timezone(zval *tz1, zval *tz2);

/* {{{ Module struct */
Expand Down Expand Up @@ -1505,45 +1505,6 @@ static void create_date_period_interval(timelib_rel_time *interval, zval *zv)
}
}

static void write_date_period_property(zend_object *obj, const char *name, const size_t name_len, zval *zv)
{
zend_string *property_name = zend_string_init(name, name_len, 0);

zend_std_write_property(obj, property_name, zv, NULL);

zval_ptr_dtor(zv);
zend_string_release(property_name);
}

static void initialize_date_period_properties(php_period_obj *period_obj)
{
zval zv;

/* rebuild properties */
zend_std_get_properties_ex(&period_obj->std);

create_date_period_datetime(period_obj->start, period_obj->start_ce, &zv);
write_date_period_property(&period_obj->std, "start", sizeof("start") - 1, &zv);

create_date_period_datetime(period_obj->current, period_obj->start_ce, &zv);
write_date_period_property(&period_obj->std, "current", sizeof("current") - 1, &zv);

create_date_period_datetime(period_obj->end, period_obj->start_ce, &zv);
write_date_period_property(&period_obj->std, "end", sizeof("end") - 1, &zv);

create_date_period_interval(period_obj->interval, &zv);
write_date_period_property(&period_obj->std, "interval", sizeof("interval") - 1, &zv);

ZVAL_LONG(&zv, (zend_long) period_obj->recurrences);
write_date_period_property(&period_obj->std, "recurrences", sizeof("recurrences") - 1, &zv);

ZVAL_BOOL(&zv, period_obj->include_start_date);
write_date_period_property(&period_obj->std, "include_start_date", sizeof("include_start_date") - 1, &zv);

ZVAL_BOOL(&zv, period_obj->include_end_date);
write_date_period_property(&period_obj->std, "include_end_date", sizeof("include_end_date") - 1, &zv);
}

/* define an overloaded iterator structure */
typedef struct {
zend_object_iterator intern;
Expand Down Expand Up @@ -1663,10 +1624,7 @@ static void date_period_it_move_forward(zend_object_iterator *iter)
zend_std_get_properties_ex(&object->std);

create_date_period_datetime(object->current, object->start_ce, &current_zv);
zend_string *property_name = ZSTR_INIT_LITERAL("current", 0);
zend_std_write_property(&object->std, property_name, &current_zv, NULL);
zval_ptr_dtor(&current_zv);
zend_string_release(property_name);

iterator->current_index++;
date_period_it_invalidate_current(iter);
Expand Down Expand Up @@ -1839,6 +1797,7 @@ static void date_register_classes(void) /* {{{ */
date_object_handlers_period.get_property_ptr_ptr = date_period_get_property_ptr_ptr;
date_object_handlers_period.read_property = date_period_read_property;
date_object_handlers_period.write_property = date_period_write_property;
date_object_handlers_period.get_properties_for = date_period_get_properties_for;

date_ce_date_error = register_class_DateError(zend_ce_error);
date_ce_date_object_error = register_class_DateObjectError(date_ce_date_error);
Expand Down Expand Up @@ -5138,8 +5097,6 @@ static bool date_period_init_finish(php_period_obj *dpobj, zend_long options, ze

dpobj->initialized = 1;

initialize_date_period_properties(dpobj);

return true;
}

Expand Down Expand Up @@ -5843,8 +5800,6 @@ static bool php_date_period_initialize_from_hash(php_period_obj *period_obj, Has

period_obj->initialized = 1;

initialize_date_period_properties(period_obj);

return 1;
} /* }}} */

Expand Down Expand Up @@ -5970,6 +5925,38 @@ PHP_METHOD(DatePeriod, __wakeup)
/* {{{ date_period_read_property */
static zval *date_period_read_property(zend_object *object, zend_string *name, int type, void **cache_slot, zval *rv)
{
if (date_period_is_internal_property(name)) {
if (type == BP_VAR_IS || type == BP_VAR_R) {
php_period_obj *period_obj = php_period_obj_from_obj(object);

if (zend_string_equals_literal(name, "start")) {
create_date_period_datetime(period_obj->start, period_obj->start_ce, rv);
return rv;
} else if (zend_string_equals_literal(name, "current")) {
create_date_period_datetime(period_obj->current, period_obj->start_ce, rv);
return rv;
} else if (zend_string_equals_literal(name, "end")) {
create_date_period_datetime(period_obj->end, period_obj->start_ce, rv);
return rv;
} else if (zend_string_equals_literal(name, "interval")) {
create_date_period_interval(period_obj->interval, rv);
return rv;
} else if (zend_string_equals_literal(name, "recurrences")) {
ZVAL_LONG(rv, period_obj->recurrences);
return rv;
} else if (zend_string_equals_literal(name, "include_start_date")) {
ZVAL_BOOL(rv, period_obj->include_start_date);
return rv;
} else if (zend_string_equals_literal(name, "include_end_date")) {
ZVAL_BOOL(rv, period_obj->include_end_date);
return rv;
}
} else {
zend_readonly_property_modification_error_ex("DatePeriod", ZSTR_VAL(name));
return &EG(uninitialized_zval);
}
}

if (type != BP_VAR_IS && type != BP_VAR_R) {
if (date_period_is_internal_property(name)) {
zend_readonly_property_modification_error_ex("DatePeriod", ZSTR_VAL(name));
Expand Down Expand Up @@ -6000,3 +5987,27 @@ static zval *date_period_get_property_ptr_ptr(zend_object *object, zend_string *

return zend_std_get_property_ptr_ptr(object, name, type, cache_slot);
}

static HashTable *date_period_get_properties_for(zend_object *object, zend_prop_purpose purpose)
{
switch (purpose) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit confused by the implementation: why this switch is necessary here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I copy-pasted this piece of code. I'm not sure why Nikita added the switch there... but after a closer examination, it seemed that ZEND_PROP_PURPOSE_GET_OBJECT_VARS is missing from the cases 🤔 I don't see the reason... but in any case, I now removed the switch case so that the custom handler kicks in in all cases now.

case ZEND_PROP_PURPOSE_DEBUG:
case ZEND_PROP_PURPOSE_SERIALIZE:
case ZEND_PROP_PURPOSE_VAR_EXPORT:
case ZEND_PROP_PURPOSE_JSON:
case ZEND_PROP_PURPOSE_ARRAY_CAST:
break;
default:
return zend_std_get_properties_for(object, purpose);
}

php_period_obj *period_obj = php_period_obj_from_obj(object);
HashTable *props = zend_array_dup(zend_std_get_properties(object));
if (!period_obj->initialized) {
return props;
}

date_period_object_to_hash(period_obj, props);

return props;
}
35 changes: 28 additions & 7 deletions ext/date/php_date.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -673,19 +673,40 @@ class DatePeriod implements IteratorAggregate
/** @cvalue PHP_DATE_PERIOD_INCLUDE_END_DATE */
public const int INCLUDE_END_DATE = UNKNOWN;

/** @readonly */
/**
* @readonly
* @virtual
*/
public ?DateTimeInterface $start;
/** @readonly */
/**
* @readonly
* @virtual
*/
public ?DateTimeInterface $current;
/** @readonly */
/**
* @readonly
* @virtual
*/
public ?DateTimeInterface $end;
/** @readonly */
/**
* @readonly
* @virtual
*/
public ?DateInterval $interval;
/** @readonly */
/**
* @readonly
* @virtual
*/
public int $recurrences;
/** @readonly */
/**
* @readonly
* @virtual
*/
public bool $include_start_date;
/** @readonly */
/**
* @readonly
* @virtual
*/
public bool $include_end_date;

public static function createFromISO8601String(string $specification, int $options = 0): static {}
Expand Down
16 changes: 8 additions & 8 deletions ext/date/php_date_arginfo.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.