-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Virtual props xmlreader #15125
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
Virtual props xmlreader #15125
Conversation
56b8726 to
c3a9905
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked through all the usages of ZEND_ACC_VIRTUAL. The only thing I found was a sub-optimal error message. I don't see any other issues.
diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index 69dd3f84d3a..52fb9e304c5 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -561,7 +561,7 @@ static int is_property_visibility_changed(zend_class_entry *ce, zval *key) return 1; } else { php_error_docref(NULL, E_WARNING, - "Cannot unserialize value for hooked property %s::$%s", + "Cannot unserialize value for virtual property %s::$%s", ZSTR_VAL(existing_propinfo->ce->name), Z_STRVAL_P(key)); zval_ptr_dtor_str(key); return -1;
...e without resorting to hooks This is useful to reduce the memory usage of objects that don't actually use the backing storage. Examples are XMLReader and DOM. When the properties were added to the stubs, these objects became much much bigger, which is a waste of memory. Closes phpGH-11644. Work towards phpGH-13988.
All properties of XMLReader are virtual and therefore don't need backing storage.
c3a9905 to
fd0d738
Compare
Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>
@iluuu1994 Thanks, I rebased this and applied your patch too.
@ericmann @SakiTakamachi @NattyNarwhal I'm asking RM permission to merge this into master as this is a new internal feature (not user-facing). It adds the ability to use the virtual annotation in the stub generator to tell the engine to avoid generating backing storage for the property where the annotation is defined upon.
Tested and it seems to work and seems low-impact. I trust your judgement here for a merge.
Allow ZEND_ACC_VIRTUAL to be used to not have property backing storage without resorting to hooks
This is useful to reduce the memory usage of objects that don't actually
use the backing storage. Examples are XMLReader and DOM. When the
properties were added to the stubs, these objects became much much
bigger, which is a waste of memory.
Closes #11644.
Works towards #13988.
Use @virtual annotation in XMLReader
All properties of XMLReader are virtual and therefore don't need backing
storage.