Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove all reflection work from main process #74

Merged
merged 7 commits into from
Sep 12, 2024

Conversation

janedbal
Copy link
Member

@janedbal janedbal commented Sep 11, 2024

  • This gains performance boost at about ~20% for our codebase
    • This is because static reflection is very costy and main process need to build all the reflection (costy ast parsing) again (even though that was already done in collectors)
    • In addition, PHPStan is not designed (performance-wise) for reflection heavy work in Rule<CollectedDataNode> as the reflection's underlying ast parser is doing ton of useless work, e.g. like parsing method bodies (such work is not useless in collectors/regular rules)
    • Thus, using reflection in Rule<CollectedDataNode> is almost always bad idea
  • Contains also BC break of changed EntrypointProvider interface.
    • This is needed to maintain the Symfony DIC constructors analysis precision. We do not have ClassHierarchy available in EntrypointProvider anymore and we need to be able to mark even children's methods as entrypoints.
    • Users can extend SimpleMethodEntrypointProvider for simple transition

@janedbal janedbal force-pushed the no-reflection-in-main-process branch 2 times, most recently from 2957272 to 59457c3 Compare September 11, 2024 14:30
@janedbal janedbal marked this pull request as ready for review September 12, 2024 08:31
@janedbal janedbal merged commit 7a8b827 into master Sep 12, 2024
12 checks passed
@janedbal janedbal deleted the no-reflection-in-main-process branch September 12, 2024 10:21
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