Skip to content

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

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 0 additions & 28 deletions ext/dom/php_dom.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1361,33 +1361,6 @@ void dom_normalize (xmlNodePtr nodep)
}
/* }}} end dom_normalize */


/* {{{ void dom_set_old_ns(xmlDoc *doc, xmlNs *ns) */
void dom_set_old_ns(xmlDoc *doc, xmlNs *ns) {
xmlNs *cur;

if (doc == NULL)
return;

if (doc->oldNs == NULL) {
doc->oldNs = (xmlNsPtr) xmlMalloc(sizeof(xmlNs));
if (doc->oldNs == NULL) {
return;
}
memset(doc->oldNs, 0, sizeof(xmlNs));
doc->oldNs->type = XML_LOCAL_NAMESPACE;
doc->oldNs->href = xmlStrdup(XML_XML_NAMESPACE);
doc->oldNs->prefix = xmlStrdup((const xmlChar *)"xml");
}

cur = doc->oldNs;
while (cur->next != NULL) {
cur = cur->next;
}
cur->next = ns;
}
/* }}} end dom_set_old_ns */

void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
{
xmlNsPtr nsptr, nsdftptr, curns, prevns = NULL;
Expand All @@ -1407,7 +1380,6 @@ void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
} else {
prevns->next = nsdftptr;
}
dom_set_old_ns(doc, curns);
curns = prevns;
}
}
Expand Down
1 change: 0 additions & 1 deletion ext/dom/php_dom.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ void php_dom_throw_error_with_message(int error_code, char *error_message, int s
void node_list_unlink(xmlNodePtr node);
int dom_check_qname(char *qname, char **localname, char **prefix, int uri_len, int name_len);
xmlNsPtr dom_get_ns(xmlNodePtr node, char *uri, int *errorcode, char *prefix);
void dom_set_old_ns(xmlDoc *doc, xmlNs *ns);
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep);
xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName);
void dom_normalize (xmlNodePtr nodep);
Expand Down
52 changes: 52 additions & 0 deletions ext/dom/tests/bug79700.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
--TEST--
dom: Can we go without dom_set_old_ns?
--FILE--
<?php

$dom = new DOMDocument();
$dom->appendChild($element = $dom->createElement('xml:xml'));
$element->setAttribute('xmlns', 'http://www.w3.org/2000/xmlns/');
$element->setAttribute('xmlns:xml', 'http://www.w3.org/XML/1998/namespace');
echo $dom->saveXML();

$html = new DOMDocument();
$element->setAttribute('xmlns', 'http://www.w3.org/2000/xmlns/');
$element->setAttribute('xmlns:xml', 'http://www.w3.org/XML/1998/namespace');
$html->appendChild($element = $html->createElement('xml:html'));

echo $html->saveHTML();

$dom->documentElement->appendChild($dom->importNode($element));

echo $dom->saveXML();

// now test performance of 10000 namespaced elements. second run should not take more than 3x (safety margin) as long as first time.

$dom = new DOMDocument();
$root = $dom->createElementNS('http://www.w3.org/2000/xhtml', 'html');
$dom->appendChild($root);

$s = microtime(true);
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps better use hrtime() instead?

for ($i = 0; $i < 10000; $i++) {
$element = $dom->createElementNS('http://www.w3.org/2000/xhtml', 'p', 'Hello World');
$root->appendChild($element);
}
$firstRun = microtime(true) - $s;

$s = microtime(true);
for ($i = 0; $i < 10000; $i++) {
$element = $dom->createElementNS('http://www.w3.org/2000/xhtml', 'p', 'Hello World');
$root->appendChild($element);
}
$secondRun = microtime(true) - $s;

if ($firstRun * 3 < $secondRun) {
echo "FAIL; second run took more than 3 times as long as first run. Performance should be linear\n";
}

--EXPECTF--
<?xml version="1.0"?>
<xml:xml xmlns="http://www.w3.org/2000/xmlns/" xmlns:xml="http://www.w3.org/XML/1998/namespace"/>
<xml:html></xml:html>
<?xml version="1.0"?>
<xml:xml xmlns="http://www.w3.org/2000/xmlns/" xmlns:xml="http://www.w3.org/XML/1998/namespace"><xml:html/></xml:xml>