Skip to content

[Performance] Short-circuit NodeComparator::areNodesEqual before printing - #8752

Merged
TomasVotruba merged 2 commits into
mainfrom
tv-perf-5
Oct 21, 2026
Merged

TomasVotruba merged 2 commits into
mainfrom
tv-perf-5

Conversation

@TomasVotruba

@TomasVotruba TomasVotruba commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Adds two cheap early bail-outs to areNodesEqual() before the expensive printWithoutComments() round-trip:

  • return false when either node is null - also avoids calling the printer on a null node
  • return false when the two single nodes are of a different class - nodes of different classes can never print as equal, so the print+compare is pure overhead

Behavior is unchanged; only the common unequal case gets faster.

Benchmark

bench/node_comparator_short_circuit_bench.php - 1.8M calls over a realistic blend of pair types (null, different-class, same-class equal/unequal), CPU time (user+sys), warmup excluded, 3 runs each branch:

us/call total CPU
before ~10.62 19.1 s
after ~6.29 11.3 s

~1.69x faster, ~41% CPU cut on this blend (5 of 9 pairs now skip the print). The win scales with how many hot-path pairs are null or different-class; same-class pairs are unchanged.

…ting

Bail out early in areNodesEqual() before the expensive printWithoutComments() call:

- return false when either node is null (also avoids printing a null node)
- return false when the two single nodes are of a different class, since nodes of different classes can never print as equal

This skips the print+compare round-trip on the common unequal case.
@TomasVotruba TomasVotruba changed the title [DeadCode] Retarget RemoveParentDelegatingConstructorRector to Class_ node (#8751) [Performance] Short-circuit NodeComparator::areNodesEqual before printing Oct 21, 2026
@TomasVotruba
TomasVotruba merged commit 2ae4680 into main Oct 21, 2026
49 checks passed
@TomasVotruba
TomasVotruba deleted the tv-perf-5 branch October 21, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant