Skip to content

perf: skip dedup map in FindNodes#576

Open
jvoisin wants to merge 1 commit into
PuerkitoBio:masterfrom
jvoisin:chemin_de_traverse
Open

perf: skip dedup map in FindNodes#576
jvoisin wants to merge 1 commit into
PuerkitoBio:masterfrom
jvoisin:chemin_de_traverse

Conversation

@jvoisin

@jvoisin jvoisin commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

FindNodes went through mapNodes, which builds a map[*html.Node]bool to deduplicate results merged across source nodes, and wrapped every match in a one-element []*html.Node slice. Both are wasteful here: each source node maps to at most itself (when it is a descendant of the current selection), so distinct source nodes can never collide. Append the matching nodes directly into the result instead.

Duplicate input nodes still have to be discarded so the resulting Selection stays a proper set, which mapNodes previously handled via appendWithoutDuplicates. A cheap isInSlice check against the (small) result slice preserves that, and is placed before the more expensive sliceContains tree walk so duplicate inputs short-circuit early.

benchstat -count=12:

           |   old    |             new
           |  sec/op  |   sec/op     vs base

FindNodes-8 | 232.6µ | 131.0µ -43.68% (p=0.000 n=12)

           |   old    |             new
           |   B/op   |      B/op      vs base

FindNodes-8 | 12.023Ki | 2.164Ki -82.00% (p=0.000 n=12)

           |   old    |            new
           | allocs/op| allocs/op   vs base

FindNodes-8 | 85.000 | 9.000 -89.41% (p=0.000 n=12)

FindNodes went through mapNodes, which builds a map[*html.Node]bool to
deduplicate results merged across source nodes, and wrapped every match in a
one-element []*html.Node slice. Both are wasteful here: each source node maps
to at most itself (when it is a descendant of the current selection), so
distinct source nodes can never collide. Append the matching nodes directly
into the result instead.

Duplicate *input* nodes still have to be discarded so the resulting Selection
stays a proper set, which mapNodes previously handled via
appendWithoutDuplicates. A cheap isInSlice check against the (small) result
slice preserves that, and is placed before the more expensive sliceContains
tree walk so duplicate inputs short-circuit early.

benchstat -count=12:

               |   old    |             new
               |  sec/op  |   sec/op     vs base
FindNodes-8    | 232.6µ   | 131.0µ       -43.68% (p=0.000 n=12)

               |   old    |             new
               |   B/op   |      B/op      vs base
FindNodes-8    | 12.023Ki |  2.164Ki      -82.00% (p=0.000 n=12)

               |   old    |            new
               | allocs/op| allocs/op   vs base
FindNodes-8    | 85.000   |  9.000       -89.41% (p=0.000 n=12)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant