Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix #79700: Bad performance with namespaced nodes due to wrong libxml assumption #11376

Closed
wants to merge 4 commits into from
Closed

Fix #79700: Bad performance with namespaced nodes due to wrong libxml assumption #11376

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
75b58bd
Use a prepending strategy instead of appending in dom_set_old_ns()
nielsdos Jun 5, 2023
84284a4
Reuse namespaces from doc->oldNs if possible in dom_get_ns()
nielsdos Jun 5, 2023
726beeb
Add a test for reconciling a reused namespace
nielsdos Jun 7, 2023
4547312
Explain why there can't be a cycle between oldNs and nsDef
nielsdos Jun 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 24 additions & 8 deletions ext/dom/php_dom.c
View file
Open in desktop
Expand Up @@ -1511,22 +1511,38 @@ NAMESPACE_ERR: Raised if

/* {{{ xmlNsPtr dom_get_ns(xmlNodePtr nodep, char *uri, int *errorcode, char *prefix) */
xmlNsPtr dom_get_ns(xmlNodePtr nodep, char *uri, int *errorcode, char *prefix) {
xmlNsPtr nsptr = NULL;

*errorcode = 0;
xmlNsPtr nsptr;

if (! ((prefix && !strcmp (prefix, "xml") && strcmp(uri, (char *)XML_XML_NAMESPACE)) ||
(prefix && !strcmp (prefix, "xmlns") && strcmp(uri, (char *)DOM_XMLNS_NAMESPACE)) ||
(prefix && !strcmp(uri, (char *)DOM_XMLNS_NAMESPACE) && strcmp (prefix, "xmlns")))) {
/* Reuse the old namespaces from doc->oldNs if possible, before creating a new one.
* This will prevent the oldNs list from growing with duplicates. */
xmlDocPtr doc = nodep->doc;
if (doc && doc->oldNs != NULL) {
nsptr = doc->oldNs;
do {
if (xmlStrEqual(nsptr->prefix, (xmlChar *)prefix) && xmlStrEqual(nsptr->href, (xmlChar *)uri)) {
goto out;
}
Comment on lines +1528 to +1530
Copy link
Member

Choose a reason for hiding this comment

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

Is this because there is nothing to do? As the node is "equal"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Note that the loop variable is the same as the return value. So when we found an exact match in the loop the loop variable is returned.

nsptr = nsptr->next;
} while (nsptr);
}
/* Couldn't reuse one, create a new one. */
nsptr = xmlNewNs(nodep, (xmlChar *)uri, (xmlChar *)prefix);
if (UNEXPECTED(nsptr == NULL)) {
goto err;
}
} else {
goto err;
}

if (nsptr == NULL) {
*errorcode = NAMESPACE_ERR;
}

out:
*errorcode = 0;
return nsptr;

err:
*errorcode = NAMESPACE_ERR;
return NULL;
}
/* }}} end dom_get_ns */

Expand Down