Make :nth-* pseudo-classes linear on wide sibling lists

CHANGELOG.md

@@ -34,6 +34,7 @@ QueryPath Changelog
 - Fix `QPXML::pi($prefix)` always returning `NULL` with an "Undefined property" warning instead of the matching processing instruction's text (issue #79)
 - Fix fatal error when running a CSS selector against a match set that contains non-element nodes (text, comment, CDATA or processing instruction), as `contents()` returns. Those nodes now simply do not match, instead of calling element-only DOM methods on them
 - Fix fatal error in `attr()`, `addClass()`, `removeClass()`, `css()`, `hasClass()`, `hasAttr()` and `removeAttr()` when the match set contains non-element nodes (text, comment, CDATA or processing instruction), as `contents()` returns (issue #98). As in jQuery, those nodes are skipped, and the `attr()` and `css()` getters return `NULL` when the first match is not an element
+- Fix `:nth-child()`, `:nth-last-child()`, `:nth-of-type()` and `:nth-last-of-type()` taking quadratic time on wide sibling lists (issue #84). Each sibling list is now indexed once per query, shared with nested `:not()` and `:has()` queries and across the per-element tests that `closest()`, `nextUntil()` and the other walk-and-stop methods run, instead of being walked for every element. `tr:nth-child(2n)` on a 4000-row table drops from about 320 ms to 4 ms
 - Fix `find('a > b')` throwing a `TypeError` when the left-hand side matched the root element, whose parent is the document rather than an element
 - Fix `QPXML::createElement()` emitting an "Only variables should be passed by reference" notice for a namespace-prefixed element name without an explicit namespace URI (issue #82)
 - Fix the `:text` pseudo-class so it matches jQuery: it selects `input` elements whose `type` attribute is absent or is `text` (case-insensitively). It never indicated, and still does not indicate, whether a node is a text node

src/CSS/DOMTraverser.php

@@ -105,10 +105,16 @@ class DOMTraverser implements Traverser
 	 * @param SplObjectStorage $splos
 	 * @param bool             $initialized
 	 * @param null             $scopeNode
+	 * @param PseudoClass|null $psHandler
+	 *   Share an outer query's handler, so a nested query reuses its sibling index.
 	 */
-	public function __construct(SplObjectStorage $splos, bool $initialized = false, $scopeNode = null)
-	{
-		$this->psHandler   = new PseudoClass();
+	public function __construct(
+		SplObjectStorage $splos,
+		bool $initialized = false,
+		$scopeNode = null,
+		?PseudoClass $psHandler = null
+	) {
+		$this->psHandler   = $psHandler ?? new PseudoClass();
 		$this->initialized = $initialized;
 
 		// Re-use the initial splos
@@ -167,15 +173,19 @@ public function find($selector): DOMTraverser
 		//$selector = $handler->toArray();
 		$found      = $this->newMatches();
 		$isFiltered = false;
-		foreach ($handler as $selectorGroup) {
-			if ($this->groupIsSetFiltered($selectorGroup)) {
-				$isFiltered = true;
-			}
+		$this->psHandler->beginQuery();
+		try {
+			foreach ($handler as $selectorGroup) {
+				if ($this->groupIsSetFiltered($selectorGroup)) {
+					$isFiltered = true;
+				}
 
-			foreach ($this->resolveGroup($selectorGroup) as $candidate) {
-				// $this->debug('Attaching ' . $candidate->nodeName);
-				$found->offsetSet($candidate);
+				foreach ($this->resolveGroup($selectorGroup) as $candidate) {
+					$found->offsetSet($candidate);
+				}
 			}
+		} finally {
+			$this->psHandler->endQuery();
 		}
 
 		// Comma-separated groups are filtered independently (as in jQuery), so the
@@ -326,7 +336,7 @@ protected function applySetFilter(array $nodes, array $pseudoClass): array
 			return [];
 		}
 
-		$traverser = new self($this->toMatches($nodes), true, $this->scopeNode);
+		$traverser = new self($this->toMatches($nodes), true, $this->scopeNode, $this->psHandler);
 		$inner     = $traverser->find($value)->matches();
 
 		$filtered = [];

src/CSS/DOMTraverser/PseudoClass.php

@@ -17,6 +17,7 @@
 use QueryPath\CSS\NotImplementedException;
 use QueryPath\CSS\EventHandler;
 use QueryPath\CSS\ParseException;
+use DOMNode;
 use SPLObjectStorage;
 
 /**
@@ -26,6 +27,51 @@
 class PseudoClass
 {
 
+	/**
+	 * Element positions within their sibling lists, keyed by element.
+	 *
+	 * Each entry is [fromStart, fromEnd, ofTypeFromStart, ofTypeFromEnd].
+	 * Positions are only kept while a query is running: the DOM may be mutated
+	 * between queries, so they must not outlive the query that indexed them.
+	 *
+	 * @var SplObjectStorage|null
+	 */
+	protected $positions;
+
+	/**
+	 * How many queries (including nested :not() and :has() queries) are running.
+	 *
+	 * @var int
+	 */
+	protected $queryDepth = 0;
+
+	/**
+	 * Parsed an+b rules, keyed by the rule string.
+	 *
+	 * @var array
+	 */
+	protected $anbRules = [];
+
+	/**
+	 * Start remembering sibling positions, so the :nth-* pseudo-classes index
+	 * each sibling list once rather than walking it for every element.
+	 *
+	 * Calls nest; positions are forgotten when the outermost query ends.
+	 */
+	public function beginQuery(): void
+	{
+		if ($this->queryDepth++ === 0) {
+			$this->positions = new SplObjectStorage();
+		}
+	}
+
+	public function endQuery(): void
+	{
+		if (--$this->queryDepth === 0) {
+			$this->positions = null;
+		}
+	}
+
 	/**
 	 * Tests whether the given element matches the given pseudoclass.
 	 *
@@ -352,7 +398,7 @@ protected function has($node, $selector): bool
 	{
 		$splos = new SPLObjectStorage();
 		$splos->offsetSet($node);
-		$traverser = new DOMTraverser($splos, true);
+		$traverser = new DOMTraverser($splos, true, null, $this);
 		$results   = $traverser->find($selector)->matches();
 
 		return count($results) > 0;
@@ -369,42 +415,24 @@ protected function isNot($node, $selector): bool
 	}
 
 	/**
-	 * Get the relative position of a node in its sibling set.
+	 * Record the position of every element child of $parent.
 	 */
-	protected function nodePositionFromStart($node, $byType = false): int
+	protected function indexSiblings(DOMNode $parent, SplObjectStorage $positions): void
 	{
-		$i   = 1;
-		$tag = $node->tagName;
-		while (isset($node->previousSibling)) {
-			$node = $node->previousSibling;
-			if ($node->nodeType === XML_ELEMENT_NODE && ( ! $byType || $node->tagName === $tag)) {
-				++$i;
+		$elements   = [];
+		$typeCounts = [];
+		foreach ($parent->childNodes as $child) {
+			if ($child->nodeType === XML_ELEMENT_NODE) {
+				$tag              = $child->tagName;
+				$typeCounts[$tag] = ($typeCounts[$tag] ?? 0) + 1;
+				$elements[]       = [$child, $typeCounts[$tag]];
 			}
 		}
 
-		return $i;
-	}
-
-	/**
-	 * Get the relative position of a node in its sibling set.
-	 *
-	 * @param      $node
-	 * @param bool $byType
-	 *
-	 * @return int
-	 */
-	protected function nodePositionFromEnd($node, $byType = false): int
-	{
-		$i   = 1;
-		$tag = $node->tagName;
-		while (isset($node->nextSibling)) {
-			$node = $node->nextSibling;
-			if ($node->nodeType === XML_ELEMENT_NODE && ( ! $byType || $node->tagName === $tag)) {
-				++$i;
-			}
+		$count = count($elements);
+		foreach ($elements as $i => [$child, $typePos]) {
+			$positions[$child] = [$i + 1, $count - $i, $typePos, $typeCounts[$child->tagName] - $typePos + 1];
 		}
-
-		return $i;
 	}
 
 	/**
@@ -433,7 +461,10 @@ protected function nodePositionFromEnd($node, $byType = false): int
 	 */
 	protected function isNthChild($node, $value, $reverse = false, $byType = false): bool
 	{
-		[$groupSize, $elementInGroup] = Util::parseAnB($value);
+		if (! isset($this->anbRules[$value])) {
+			$this->anbRules[$value] = Util::parseAnB($value);
+		}
+		[$groupSize, $elementInGroup] = $this->anbRules[$value];
 		$parent = $node->parentNode;
 		if (empty($parent)
 			|| ($groupSize === 0 && $elementInGroup === 0)
@@ -442,11 +473,12 @@ protected function isNthChild($node, $value, $reverse = false, $byType = false):
 		}
 
 		// First we need to find the position of $node in other elements.
-		if ($reverse) {
-			$pos = $this->nodePositionFromEnd($node, $byType);
-		} else {
-			$pos = $this->nodePositionFromStart($node, $byType);
+		// Outside a query nothing is remembered, so index into a throwaway store.
+		$positions = $this->positions ?? new SplObjectStorage();
+		if (! $positions->offsetExists($node)) {
+			$this->indexSiblings($parent, $positions);
 		}
+		$pos = $positions[$node][($byType ? 2 : 0) + ($reverse ? 1 : 0)];
 
 		// If group size is 0, we just check to see if this
 		// is the nth element:

src/Helpers/NodeMatcher.php

@@ -4,6 +4,7 @@
 
 use DOMNode;
 use QueryPath\CSS\DOMTraverser;
+use QueryPath\CSS\DOMTraverser\PseudoClass;
 use QueryPath\CSS\ParseException;
 use SplObjectStorage;
 
@@ -20,9 +21,9 @@
  * would be the only member of its own one-element set.
  *
  * matches() remembers each per-node answer, because the walk-and-stop traversals
- * (closest(), nextUntil(), ...) revisit the same nodes from every starting element. An
- * instance must not outlive the call that made it, or a later DOM change could leave it
- * holding stale answers.
+ * (closest(), nextUntil(), ...) revisit the same nodes from every starting element, and
+ * every query shares one sibling index for the :nth-* pseudo-classes. An instance must not
+ * outlive the call that made it, or a later DOM change could leave it holding stale answers.
  */
 final class NodeMatcher
 {
@@ -37,6 +38,11 @@ final class NodeMatcher
 	 */
 	private $results;
 
+	/**
+	 * @var PseudoClass
+	 */
+	private $psHandler;
+
 	/**
 	 * @param string $selector
 	 *   A valid CSS selector.
@@ -45,6 +51,10 @@ public function __construct($selector)
 	{
 		$this->selector = $selector;
 		$this->results  = new SplObjectStorage();
+
+		// Held open for the matcher's lifetime, which is a single call.
+		$this->psHandler = new PseudoClass();
+		$this->psHandler->beginQuery();
 	}
 
 	/**
@@ -124,7 +134,7 @@ public function matching(SplObjectStorage $nodes): SplObjectStorage
 		// than seeding them with a descendant search. The scope node is left at its default (the
 		// document element) so that `:scope` means the same here as in find(); scoping to a
 		// candidate would make every candidate match `:scope`.
-		$traverser = new DOMTraverser($nodes, true);
+		$traverser = new DOMTraverser($nodes, true, null, $this->psHandler);
 
 		return $traverser->find($this->selector)->matches();
 	}

tests/QueryPath/CSS/DOMTraverserTest.php

@@ -37,10 +37,14 @@ public function testConstructor()
 		$this->assertInstanceOf('\QueryPath\CSS\DOMTraverser', $traverser);
 	}
 
-	protected function traverser()
+	protected function traverser($xml = null)
 	{
 		$dom = new DOMDocument('1.0');
-		$dom->load($this->xml_file);
+		if ($xml === null) {
+			$dom->load($this->xml_file);
+		} else {
+			$dom->loadXML($xml);
+		}
 
 		$splos = new SPLObjectStorage();
 		$splos->offsetSet($dom->documentElement);
@@ -368,4 +372,107 @@ public function testMultipleSelectors()
 		$matches = $this->find('one, two');
 		$this->assertEquals(2, count($matches));
 	}
+
+	/**
+	 * The <ul> children are, by position: l1 p1 l2 l3 p2 l4 l5 p3.
+	 */
+	protected $nthXml = '<?xml version="1.0"?><root><ul id="ul">'
+		. 'text<!-- comment --><li id="l1"/><p id="p1"/><li id="l2"/><!-- comment --><li id="l3"/>'
+		. 'text<p id="p2"/><li id="l4"/><li id="l5"/><p id="p3"/>text'
+		. '</ul><ol id="ol"><li id="o1"/><li id="o2"/></ol></root>';
+
+	protected function ids($nodes): array
+	{
+		$ids = [];
+		foreach ($nodes as $node) {
+			$ids[] = $node->getAttribute('id');
+		}
+
+		return $ids;
+	}
+
+	public function nthPseudoClassProvider(): array
+	{
+		return [
+			// :nth-child()
+			['ul > :nth-child(odd)', ['l1', 'l2', 'p2', 'l5']],
+			['ul > :nth-child(even)', ['p1', 'l3', 'l4', 'p3']],
+			['ul > :nth-child(n)', ['l1', 'p1', 'l2', 'l3', 'p2', 'l4', 'l5', 'p3']],
+			['ul > :nth-child(3)', ['l2']],
+			['ul > :nth-child(-n+3)', ['l1', 'p1', 'l2']],
+			['ul > :nth-child(3n-5)', ['l1', 'l3', 'l5']],
+			['ul > :nth-child(0n+0)', []],
+			['ul > :nth-child(-2n-1)', []],
+			['ul > :nth-child(9)', []],
+			['li:nth-child(2n+1)', ['l1', 'l2', 'l5', 'o1']],
+
+			// :nth-last-child()
+			['ul > :nth-last-child(1)', ['p3']],
+			['ul > :nth-last-child(odd)', ['p1', 'l3', 'l4', 'p3']],
+			['ul > :nth-last-child(-n+2)', ['l5', 'p3']],
+			['ul > :nth-last-child(8)', ['l1']],
+			['li:nth-last-child(2)', ['l5', 'o1']],
+
+			// :nth-of-type(). The <li> are l1-l5 and the <p> p1-p3.
+			['li:nth-of-type(odd)', ['l1', 'l3', 'l5', 'o1']],
+			['p:nth-of-type(2)', ['p2']],
+			['ul > :nth-of-type(2)', ['l2', 'p2']],
+			['ul > :nth-of-type(-n+1)', ['l1', 'p1']],
+			['ul > :nth-of-type(4)', ['l4']],
+
+			// :nth-last-of-type()
+			['li:nth-last-of-type(1)', ['l5', 'o2']],
+			['ul > :nth-last-of-type(2)', ['p2', 'l4']],
+			['p:nth-last-of-type(n+2)', ['p1', 'p2']],
+			['ul > :nth-last-of-type(3n)', ['p1', 'l3']],
+
+			// Several :nth-* in one query share the same sibling index.
+			['li:nth-child(odd):nth-of-type(odd)', ['l1', 'l5', 'o1']],
+			['ul > :nth-child(even):nth-last-of-type(1)', ['p3']],
+			['ul > :nth-child(1), ul > :nth-last-child(1)', ['l1', 'p3']],
+
+			// Nested queries.
+			['ul > :not(:nth-child(odd))', ['p1', 'l3', 'l4', 'p3']],
+			['ul > :not(:nth-last-of-type(-n+2))', ['l1', 'p1', 'l2', 'l3']],
+		];
+	}
+
+	/**
+	 * @dataProvider nthPseudoClassProvider
+	 */
+	public function testNthPseudoClasses($selector, array $expected)
+	{
+		$this->assertSame($expected, $this->ids($this->traverser($this->nthXml)->find($selector)->matches()));
+	}
+
+	public function testNthPseudoClassesInFilterAndChildren()
+	{
+		$this->assertSame(['l1', 'l2', 'p2', 'l5'], $this->ids(qp($this->nthXml, 'ul > *')->filter(':nth-child(odd)')->get()));
+		$this->assertSame(['l5', 'p3'], $this->ids(qp($this->nthXml, 'ul')->children(':nth-last-of-type(1)')->get()));
+		$this->assertSame(['l2', 'o2'], $this->ids(qp($this->nthXml, 'ul, ol')->children('li:nth-of-type(2)')->get()));
+	}
+
+	/**
+	 * Sibling positions are indexed per query, so querying again after the DOM
+	 * changes must see the new positions.
+	 */
+	public function testNthPseudoClassesAfterMutation()
+	{
+		$qp = qp('<?xml version="1.0"?><root><li id="a"/><li id="b"/><li id="c"/></root>');
+
+		$this->assertSame(['b'], $this->ids($qp->find('li:nth-child(2)')->get()));
+		$this->assertSame(['b'], $this->ids($qp->find('li:nth-last-of-type(2)')->get()));
+
+		$qp->find('root')->prepend('<li id="new"/>')->append('<p/>');
+
+		$this->assertSame(['a'], $this->ids($qp->find('li:nth-child(2)')->get()));
+		$this->assertSame(['c'], $this->ids($qp->find('li:nth-last-child(2)')->get()));
+		$this->assertSame(['a'], $this->ids($qp->find('li:nth-of-type(2)')->get()));
+		$this->assertSame(['b'], $this->ids($qp->find('li:nth-last-of-type(2)')->get()));
+
+		$qp->find('#b')->remove();
+
+		$this->assertSame(['c'], $this->ids($qp->find('li:nth-child(3)')->get()));
+		$this->assertSame(['new', 'c'], $this->ids($qp->find('li:nth-child(odd)')->get()));
+	}
 }