Skip to content

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

Merged
jakejackson1 merged 2 commits into
mainfrom
fix/issue-84
Oct 26, 2026
Merged

jakejackson1 merged 2 commits into
mainfrom
fix/issue-84

Conversation

@gravitypdf-bot

@gravitypdf-bot gravitypdf-bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

:nth-child(), :nth-last-child(), :nth-of-type() and :nth-last-of-type() worked out each candidate's position by walking its siblings, so matching n siblings cost about n²/2 hops.

// <table> with 4000 <tr>
qp($xml, 'tr:nth-child(2n)');   // before: ~320 ms   after: ~4 ms

What changed

  • PseudoClass indexes a parent's element children once. For each child it records the position from the start and from the end, both overall and among same-tag siblings, so all four families read from one table.
  • The index only lives while a query runs. DOMTraverser::find() brackets traversal with beginQuery() / endQuery() in a try/finally. Calls nest, and positions are dropped when the outermost query ends, so a DOM mutated between queries is never matched against stale positions and no node references outlive the query. With no query running (calling elementMatches() directly), the sibling list is indexed into a throwaway store, so nothing is remembered and there is one position algorithm, not two.
  • Nested :not() / :has() traversers share their caller's PseudoClass through a new optional fourth DOMTraverser constructor argument. Without that, each candidate would build a fresh index, which is slower than the old walk.
  • NodeMatcher (from Match selectors against the nodes in hand, not their descendants #72) holds one PseudoClass open for its lifetime, which is a single call. Its per-node queries for closest(), nextUntil() and the other walk-and-stop methods therefore share one index. filter() and children($selector) already run one set-based query on main, so they need nothing extra.
  • The parsed an+b rule is memoised per PseudoClass instead of re-parsed for every node.

The match logic is unchanged.

Benchmarks

PHP 8.5, best of 3. The first table uses the issue's <table> with n × <tr><td>x</td></tr>.

rows tr tr:nth-child(2n) before after
500 0.3 ms 6.1 ms 0.6 ms
1000 0.6 ms 18.9 ms 1.1 ms
2000 1.2 ms 71.9 ms 2.1 ms
4000 2.8 ms 319.5 ms 4.1 ms
20000 13.8 ms 8007.9 ms 25.8 ms
2000 rows before after
tr:nth-child(2n) 73.2 ms 2.6 ms
tr:nth-last-child(2n) 81.2 ms 2.5 ms
tr:nth-of-type(2n) 124.0 ms 2.4 ms
tr:nth-last-of-type(2n) 131.2 ms 2.3 ms
tr:not(:nth-child(2n)) 103.3 ms 13.9 ms
qp($xml, 'tr')->filter(':nth-child(2n)') 89.8 ms 4.7 ms
qp($xml, 'table')->children(':nth-child(2n)') 80.6 ms 3.8 ms
qp($xml, 'tr:first-child')->nextUntil(':nth-last-child(1)') 206.0 ms 21.9 ms
qp($xml, 'td')->closest('tr:nth-child(2n)') 109.7 ms 41.9 ms

The last four rows were re-measured after rebasing onto main with #72 and #73.

Tests

  • DOMTraverserTest::testNthPseudoClasses covers all four families on a mixed li/p list with interleaved text and comments, across two parents. Cases include odd, even, n, 0n+0, -n+3, 3n-5, -2n-1, out-of-range indexes, several :nth-* in one selector, comma groups, and :not(:nth-*).
  • testNthPseudoClassesInFilterAndChildren covers filter() and children().
  • testNthPseudoClassesAfterMutation runs queries, then prepend(), append() and remove(), then queries again on the same DOMQuery.
  • In PseudoClassTest:
    • Each sibling list is indexed once per query, including through a nested :not().
    • Positions are not reused once the outermost query ends, but survive a nested query ending.
    • Nothing is cached outside a query.

vendor/bin/phpunit: 598 tests, 1736 assertions, 0 failures (1 skipped). composer run lint and composer run lint:min-php are clean.

  • No timing assertions.

Rebased onto #72 and #73

#119 used to thread the shared handler through per-element loops in filter() and children(). On main those loops are gone, so that part was dropped in favour of main's version.

There is no behaviour change. DOMTraverser::__construct() gains an optional fourth argument, and PseudoClass gains beginQuery() / endQuery().

Fixes #84.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.05%. Comparing base (6ae6acf) to head (21f088b).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #119      +/-   ##
============================================
+ Coverage     90.69%   91.05%   +0.35%     
- Complexity     1424     1431       +7     
============================================
  Files            25       26       +1     
  Lines          3170     3174       +4     
============================================
+ Hits           2875     2890      +15     
+ Misses          295      284      -11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gravitypdf-bot and others added 2 commits October 26, 2026 12:53
isNthChild() walked the preceding (or following) siblings of every
candidate, so matching n siblings cost n^2/2 hops. Positions are now
recorded for a whole sibling list on first use and remembered until
the outermost DOMTraverser::find() ends, so a DOM mutated between
queries is never answered from a stale index.

Nested :not()/:has() queries, and the per-element queries run by
filter() and children(), share the outer handler so they reuse the
same index instead of rebuilding it per candidate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
NodeMatcher::matches() runs a traverser per node for the walk-and-stop
methods, so each got its own handler and indexed the whole sibling list
to answer one node: nextUntil(':nth-last-child(1)') over 2000 siblings
went from 290 ms on main to 950 ms. The matcher now holds one handler
open for its lifetime, which is already a single call (17 ms).

Outside a query the index is built into a throwaway store, replacing the
separate sibling walkers, and entries are flat arrays.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jakejackson1
jakejackson1 merged commit 1767909 into main Oct 26, 2026
14 checks passed
@jakejackson1
jakejackson1 deleted the fix/issue-84 branch October 26, 2026 02:13
gravitypdf-bot added a commit that referenced this pull request Oct 26, 2026
findsAny() built each per-element and per-sibling traverser with its own
PseudoClass, so with #119's index every one re-indexed its sibling list:
li:has(~ li:nth-child(1)) over 400 siblings took about 9 s. It now passes
its own handler, as is() does, which also lets a nested :has() reuse the
parsed-argument cache (about 0.26 s).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jakejackson1 pushed a commit that referenced this pull request Oct 26, 2026
* Make :has() match related elements in DOMTraverser

PseudoClass::has() ran its argument in initialized mode against the
element itself, so ul:has(li) asked whether the ul is an li and never
matched. It now reads the argument as a relative selector anchored to
the element: a descendant by default, or a child, next sibling or later
sibling for a leading >, + or ~. :not() and :matches() keep testing the
element itself.

:has() is no longer a set filter when its argument is positional: the
argument is searched for around each element, as in jQuery.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test a later sibling itself against a sibling-relative :has()

find() now searches descendants only (#73), so the per-sibling search in
:has(+ ...) and :has(~ ...) no longer saw the sibling it started from.
Test the sibling itself first, and search below it only when the group
goes on to a child or descendant.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Return whether a :has() group starts at a sibling, not its combinator

has() only ever asked whether the anchor combinator was + or ~, so
relativeSelector() now answers that, and the descendant and sibling
searches become the two arms of one if/else.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Share the sibling index with the queries :has() runs

findsAny() built each per-element and per-sibling traverser with its own
PseudoClass, so with #119's index every one re-indexed its sibling list:
li:has(~ li:nth-child(1)) over 400 siblings took about 9 s. It now passes
its own handler, as is() does, which also lets a nested :has() reuse the
parsed-argument cache (about 0.26 s).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CSS :nth-child() and friends are O(n²) on sibling-list width

2 participants