Skip to content

StaticMapper: added fast path for IdentifierTypeNode#4565

Merged
samsonasik merged 6 commits intorectorphp:mainfrom
staabm:fast-identifier-mapper
Jul 21, 2023
Merged

StaticMapper: added fast path for IdentifierTypeNode#4565
samsonasik merged 6 commits intorectorphp:mainfrom
staabm:fast-identifier-mapper

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Jul 21, 2023

the idea behind this method is:

  • we see in NameImportingPostRector is slow rector#8077 (comment) that name scope creation is slow
  • IdentifierTypeMapper does not need the name-scope for its inner workings -> lets try a separate fast path in case we know the $typeNode=IdentifierTypeNode so we can skip name scope creation

Result: 1 minute faster rector runs ; additionally we are saving 11% on memory usage

grafik

closes rectorphp/rector#8077

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Jul 21, 2023

I guess the test-suite segfaults because of a infinite loop. I couldn't identify yet why this happens. I can also see it locally but wanted to verify in CI whether the problem also happens

@staabm staabm marked this pull request as ready for review July 21, 2023 09:42
@staabm staabm requested a review from TomasVotruba as a code owner July 21, 2023 09:42
@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Jul 21, 2023

updated the initial description with our results. with this change rector runs 23% faster in our use-case which means saving 1 minute on a 4 minute job

additionally we are saving 11% on memory usage

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Jul 21, 2023

I think we are good to go

@samsonasik
Copy link
Copy Markdown
Member

Let's give it a try, thank you @staabm

@samsonasik samsonasik merged commit efce1c7 into rectorphp:main Jul 21, 2023
@staabm staabm deleted the fast-identifier-mapper branch July 21, 2023 10: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.

NameImportingPostRector is slow

2 participants