From d8417564730f45370d5192d02c7a1180dbd11dd2 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 29 May 2023 22:44:31 +0200 Subject: [PATCH 1/3] Remove unnecessary tree setting in dom_zvals_to_fragment() This is already done by xmlNewDocText(). --- ext/dom/parentnode.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index b7a7cfd07f38..ea6956c5bd6f 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -217,8 +217,6 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod } else if (Z_TYPE(nodes[i]) == IS_STRING) { newNode = xmlNewDocText(documentNode, (xmlChar *) Z_STRVAL(nodes[i])); - xmlSetTreeDoc(newNode, documentNode); - if (!xmlAddChild(fragment, newNode)) { xmlFreeNode(newNode); goto hierarchy_request_err; From b5f1373d0ac19d8e81bc5c7c6f03bd0dd4ff0218 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 29 May 2023 23:14:27 +0200 Subject: [PATCH 2/3] Implement dom_get_doc_props_read_only() I was surprised to see that getting the stricterror property showed in in the Callgrind profile of some tests. Turns out we sometimes allocate them. Fix this by returning the default in case no changes were made yet. --- ext/dom/document.c | 31 +++++++++++------------------ ext/dom/php_dom.c | 49 ++++++++++++++++++++++++---------------------- ext/dom/php_dom.h | 1 + 3 files changed, 38 insertions(+), 43 deletions(-) diff --git a/ext/dom/document.c b/ext/dom/document.c index c60198a3be11..13324645e987 100644 --- a/ext/dom/document.c +++ b/ext/dom/document.c @@ -297,7 +297,7 @@ readonly=no int dom_document_format_output_read(dom_object *obj, zval *retval) { if (obj->document) { - dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document); + libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document); ZVAL_BOOL(retval, doc_prop->formatoutput); } else { ZVAL_FALSE(retval); @@ -322,7 +322,7 @@ readonly=no int dom_document_validate_on_parse_read(dom_object *obj, zval *retval) { if (obj->document) { - dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document); + libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document); ZVAL_BOOL(retval, doc_prop->validateonparse); } else { ZVAL_FALSE(retval); @@ -347,7 +347,7 @@ readonly=no int dom_document_resolve_externals_read(dom_object *obj, zval *retval) { if (obj->document) { - dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document); + libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document); ZVAL_BOOL(retval, doc_prop->resolveexternals); } else { ZVAL_FALSE(retval); @@ -372,7 +372,7 @@ readonly=no int dom_document_preserve_whitespace_read(dom_object *obj, zval *retval) { if (obj->document) { - dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document); + libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document); ZVAL_BOOL(retval, doc_prop->preservewhitespace); } else { ZVAL_FALSE(retval); @@ -397,7 +397,7 @@ readonly=no int dom_document_recover_read(dom_object *obj, zval *retval) { if (obj->document) { - dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document); + libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document); ZVAL_BOOL(retval, doc_prop->recover); } else { ZVAL_FALSE(retval); @@ -422,7 +422,7 @@ readonly=no int dom_document_substitue_entities_read(dom_object *obj, zval *retval) { if (obj->document) { - dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document); + libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document); ZVAL_BOOL(retval, doc_prop->substituteentities); } else { ZVAL_FALSE(retval); @@ -1176,7 +1176,6 @@ static xmlDocPtr dom_document_parser(zval *id, int mode, char *source, size_t so { xmlDocPtr ret; xmlParserCtxtPtr ctxt = NULL; - dom_doc_propsptr doc_props; dom_object *intern; php_libxml_ref_obj *document = NULL; int validate, recover, resolve_externals, keep_blanks, substitute_ent; @@ -1189,17 +1188,13 @@ static xmlDocPtr dom_document_parser(zval *id, int mode, char *source, size_t so document = intern->document; } - doc_props = dom_get_doc_props(document); + libxml_doc_props const* doc_props = dom_get_doc_props_read_only(document); validate = doc_props->validateonparse; resolve_externals = doc_props->resolveexternals; keep_blanks = doc_props->preservewhitespace; substitute_ent = doc_props->substituteentities; recover = doc_props->recover; - if (document == NULL) { - efree(doc_props); - } - xmlInitParser(); if (mode == DOM_LOAD_FILE) { @@ -1387,7 +1382,6 @@ PHP_METHOD(DOMDocument, save) size_t file_len = 0; int bytes, format, saveempty = 0; dom_object *intern; - dom_doc_propsptr doc_props; char *file; zend_long options = 0; @@ -1405,7 +1399,7 @@ PHP_METHOD(DOMDocument, save) /* encoding handled by property on doc */ - doc_props = dom_get_doc_props(intern->document); + libxml_doc_props const* doc_props = dom_get_doc_props_read_only(intern->document); format = doc_props->formatoutput; if (options & LIBXML_SAVE_NOEMPTYTAG) { saveempty = xmlSaveNoEmptyTags; @@ -1433,7 +1427,6 @@ PHP_METHOD(DOMDocument, saveXML) xmlBufferPtr buf; xmlChar *mem; dom_object *intern, *nodeobj; - dom_doc_propsptr doc_props; int size, format, saveempty = 0; zend_long options = 0; @@ -1444,7 +1437,7 @@ PHP_METHOD(DOMDocument, saveXML) DOM_GET_OBJ(docp, id, xmlDocPtr, intern); - doc_props = dom_get_doc_props(intern->document); + libxml_doc_props const* doc_props = dom_get_doc_props_read_only(intern->document); format = doc_props->formatoutput; if (nodep != NULL) { @@ -1928,7 +1921,6 @@ PHP_METHOD(DOMDocument, saveHTMLFile) size_t file_len; int bytes, format; dom_object *intern; - dom_doc_propsptr doc_props; char *file; const char *encoding; @@ -1947,7 +1939,7 @@ PHP_METHOD(DOMDocument, saveHTMLFile) encoding = (const char *) htmlGetMetaEncoding(docp); - doc_props = dom_get_doc_props(intern->document); + libxml_doc_props const* doc_props = dom_get_doc_props_read_only(intern->document); format = doc_props->formatoutput; bytes = htmlSaveFileFormat(file, docp, encoding, format); @@ -1969,7 +1961,6 @@ PHP_METHOD(DOMDocument, saveHTML) dom_object *intern, *nodeobj; xmlChar *mem = NULL; int format; - dom_doc_propsptr doc_props; id = ZEND_THIS; if (zend_parse_parameters(ZEND_NUM_ARGS(), @@ -1980,7 +1971,7 @@ PHP_METHOD(DOMDocument, saveHTML) DOM_GET_OBJ(docp, id, xmlDocPtr, intern); - doc_props = dom_get_doc_props(intern->document); + libxml_doc_props const* doc_props = dom_get_doc_props(intern->document); format = doc_props->formatoutput; if (nodep != NULL) { diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 662c6e9ef7ce..3a92fc54c45c 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -140,6 +140,17 @@ int dom_node_children_valid(xmlNodePtr node) { } /* }}} end dom_node_children_valid */ +static const libxml_doc_props default_doc_props = { + .formatoutput = false, + .validateonparse = false, + .resolveexternals = false, + .preservewhitespace = true, + .substituteentities = false, + .stricterror = true, + .recover = false, + .classmap = NULL, +}; + /* {{{ dom_get_doc_props() */ dom_doc_propsptr dom_get_doc_props(php_libxml_ref_obj *document) { @@ -149,28 +160,31 @@ dom_doc_propsptr dom_get_doc_props(php_libxml_ref_obj *document) return document->doc_props; } else { doc_props = emalloc(sizeof(libxml_doc_props)); - doc_props->formatoutput = 0; - doc_props->validateonparse = 0; - doc_props->resolveexternals = 0; - doc_props->preservewhitespace = 1; - doc_props->substituteentities = 0; - doc_props->stricterror = 1; - doc_props->recover = 0; - doc_props->classmap = NULL; + memcpy(doc_props, &default_doc_props, sizeof(libxml_doc_props)); if (document) { document->doc_props = doc_props; } return doc_props; } } +/* }}} */ + +libxml_doc_props const* dom_get_doc_props_read_only(const php_libxml_ref_obj *document) +{ + if (document && document->doc_props) { + return document->doc_props; + } else { + return &default_doc_props; + } +} static void dom_copy_doc_props(php_libxml_ref_obj *source_doc, php_libxml_ref_obj *dest_doc) { - dom_doc_propsptr source, dest; + dom_doc_propsptr dest; if (source_doc && dest_doc) { - source = dom_get_doc_props(source_doc); + libxml_doc_props const* source = dom_get_doc_props_read_only(source_doc); dest = dom_get_doc_props(dest_doc); dest->formatoutput = source->formatoutput; @@ -212,10 +226,8 @@ void dom_set_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece zend_class_entry *dom_get_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece) { - dom_doc_propsptr doc_props; - if (document) { - doc_props = dom_get_doc_props(document); + libxml_doc_props const* doc_props = dom_get_doc_props_read_only(document); if (doc_props->classmap) { zend_class_entry *ce = zend_hash_find_ptr(doc_props->classmap, basece->name); if (ce) { @@ -230,16 +242,7 @@ zend_class_entry *dom_get_doc_classmap(php_libxml_ref_obj *document, zend_class_ /* {{{ dom_get_strict_error() */ int dom_get_strict_error(php_libxml_ref_obj *document) { - int stricterror; - dom_doc_propsptr doc_props; - - doc_props = dom_get_doc_props(document); - stricterror = doc_props->stricterror; - if (document == NULL) { - efree(doc_props); - } - - return stricterror; + return dom_get_doc_props_read_only(document)->stricterror; } /* }}} */ diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index fdfdd4e7a31c..a7ae09384cfd 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -97,6 +97,7 @@ typedef struct { dom_object *dom_object_get_data(xmlNodePtr obj); dom_doc_propsptr dom_get_doc_props(php_libxml_ref_obj *document); +libxml_doc_props const* dom_get_doc_props_read_only(const php_libxml_ref_obj *document); zend_object *dom_objects_new(zend_class_entry *class_type); zend_object *dom_nnodemap_objects_new(zend_class_entry *class_type); #ifdef LIBXML_XPATH_ENABLED From b91b9a3813e0e48857296f1b708ba662d9712891 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 29 May 2023 23:17:48 +0200 Subject: [PATCH 3/3] Update UPGRADING.INTERNALS --- UPGRADING.INTERNALS | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index b4675e22215e..99b609a115b0 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -116,6 +116,11 @@ PHP 8.3 INTERNALS UPGRADE NOTES - The PHPAPI spl_iterator_apply() function now returns zend_result instead of int. There are no functional changes. + f. ext/dom + - A new function dom_get_doc_props_read_only() is added to gather the document + properties in a read-only way. This function avoids allocation when there are + no document properties changed yet. + ======================== 4. OpCode changes ========================