Skip to content

[perf] Remove usage of CloningVisitor on AstResolver - #8861

Merged
TomasVotruba merged 2 commits into
mainfrom
remove-cloning-ast
Nov 12, 2026
Merged

TomasVotruba merged 2 commits into
mainfrom
remove-cloning-ast

Conversation

@samsonasik

Copy link
Copy Markdown
Member

AstResolver don't need changed node reprinting, so CloningVisitor is not needed. It was using NodeScopeAndMetadataDecorator which utilize CloningVisitor on final node result get.

This PR eliminate CloningVisitor on AstResolver.

Bench (bench/ast_resolver_no_cloning_bench.php):

main this PR diff
Memory held by $parsedFileNodes 31.96 MB 22.83 MB -28.6%
CPU, run 1 8.9783 s 8.6474 s
CPU, run 2 8.9107 s 8.8772 s
CPU, average 8.9445 s 8.7623 s -2.0%

@samsonasik
samsonasik marked this pull request as draft October 27, 2026 16:07
@samsonasik samsonasik closed this Oct 27, 2026
@samsonasik
samsonasik deleted the remove-cloning-ast branch October 27, 2026 16:25
@samsonasik
samsonasik restored the remove-cloning-ast branch November 12, 2026 18:03
@samsonasik samsonasik reopened this Nov 12, 2026
@samsonasik
samsonasik marked this pull request as ready for review November 12, 2026 18:04
@samsonasik

Copy link
Copy Markdown
Member Author

Re-open, this should be green now 👍

@samsonasik

Copy link
Copy Markdown
Member Author

@TomasVotruba ready 👍

@TomasVotruba
TomasVotruba merged commit 9e6021d into main Nov 12, 2026
51 checks passed
@TomasVotruba
TomasVotruba deleted the remove-cloning-ast branch November 12, 2026 18:08
@TomasVotruba

Copy link
Copy Markdown
Member

Fewer nodes to worry about, thank you 👍

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.

2 participants