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

Refactor use import resolving #4998

Merged
merged 11 commits into from Sep 12, 2023
Merged

Refactor use import resolving #4998

merged 11 commits into from Sep 12, 2023

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Sep 11, 2023

followup to #4994

reduce duplicate node traversal of the same nodes over and over again

@staabm
Copy link
Contributor Author

staabm commented Sep 12, 2023

@samsonasik do you have an idea what I missed?

@samsonasik
Copy link
Member

it may be an infinite loop somewhere

@TomasVotruba
Copy link
Member

it may be an infinite loop somewhere

Yep, the "operation canceled" usually means infinite loop. E.g. traversing nested nodes or calling 2 methods back and forth.

@staabm
Copy link
Contributor Author

staabm commented Sep 12, 2023

this looks good now. I will update the PR description after merge, because atm I cannot measure the perf diff before-merge in the codeigniter codebase

@staabm staabm marked this pull request as ready for review September 12, 2023 10:25
@@ -45,9 +46,14 @@ public function resolveFromStmts(array $stmts): array
$aliasedUses = [];

$this->useImportsTraverser->traverserStmts($stmts, static function (
int $useType,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add enum param doc type here? I think it's Use_::TYPE_*

Comment on lines 48 to +49
$this->useImportsTraverser->traverserStmts($stmts, static function (
int $useType,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this:

Suggested change
$this->useImportsTraverser->traverserStmts($stmts, static function (
int $useType,
/** @param Use_::TYPE_* $useType */
$this->useImportsTraverser->traverserStmts($stmts, static function (
int $useType,

@TomasVotruba
Copy link
Member

Thank you 👏

@TomasVotruba TomasVotruba merged commit aeba96a into rectorphp:main Sep 12, 2023
37 checks passed
@staabm staabm deleted the refactor branch September 12, 2023 13:06
@samsonasik
Copy link
Member

@TomasVotruba @staabm I tried in CodeIgniter 4 project, it now very very slow, it got 2 times slower now.

@samsonasik
Copy link
Member

@TomasVotruba @staabm hm.., I am investigating, it probably on my local dev that installing php extensions that make cli slower:

  • php82-imap
  • php82-pcntl
  • php82-imagick

I will try uninstall them and re-try

@staabm
Copy link
Contributor Author

staabm commented Sep 12, 2023

Maybe you have xdebug or something like that enabled?

@samsonasik
Copy link
Member

@staabm just verify, uninstalling php82-imap resolve it, I am wondering if I need to configure imap extension to make cli keep fast.

@samsonasik
Copy link
Member

samsonasik commented Sep 12, 2023

probably related with imap extension asked 9 years ago https://serverfault.com/questions/586141/php-cli-with-imap-5-second-startup-delay

which parallel open new 127.0.0.1 tcp server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants