Skip to content

Added regression test#3062

Merged
ondrejmirtes merged 2 commits intophpstan:1.11.xfrom
staabm:fix2
May 11, 2024
Merged

Added regression test#3062
ondrejmirtes merged 2 commits intophpstan:1.11.xfrom
staabm:fix2

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented May 10, 2024 •
edited
Loading

closes phpstan/phpstan#10979

reverting the fix for #2985 does not regress the test of said PR.
I need to dive deeper why the test no longer reproduces the perf problem we had at the time #2985 was created

I had a typo locally .. investigating..

@staabm staabm marked this pull request as draft May 10, 2024 11:28
staabm
staabm commented May 10, 2024
View reviewed changes
src/Type/TypeCombinator.php Outdated
Copy link
Copy Markdown
Contributor Author

@staabm staabm May 10, 2024

Choose a reason for hiding this comment

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

reverts #2985

staabm referenced this pull request May 10, 2024
@ondrejmirtes
ClassReflection - cache enum cases
39ce042
@staabm staabm marked this pull request as ready for review May 10, 2024 12:56
@phpstan-bot
Copy link
Copy Markdown
Collaborator

phpstan-bot commented May 10, 2024

This pull request has been marked as ready for review.

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented May 10, 2024

after this PR we now have both cases fast:

php bin/phpstan analyze --debug bug-10772.php  1.30s user 0.14s system 98% cpu 1.451 total


php bin/phpstan analyze --debug LibraryMasterEnum.php  3.16s user 0.14s system 99% cpu 3.328 total

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented May 10, 2024

will be AFK for a few hours

@staabm staabm mentioned this pull request May 10, 2024
Closed
ondrejmirtes
ondrejmirtes requested changes May 10, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

Yeah this looks good 👍

src/Type/ObjectType.php Outdated
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes May 10, 2024

Choose a reason for hiding this comment

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

Needs to be reset in resetCaches()

src/Type/ObjectType.php Outdated
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes May 10, 2024

Choose a reason for hiding this comment

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

The cache key needs to be $this->describeCache().

@staabm staabm force-pushed the fix2 branch 3 times, most recently from 69a9068 to 168d145 Compare May 10, 2024 18:15
@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented May 10, 2024 •
edited
Loading

this one is good to go. I will have a look at the other remaining open PRs after this one is merged, whether there is still meaningful impact

php bin/phpstan analyze --debug LibraryMasterEnum.php  
  2.02s user 0.13s system 99% cpu 2.160 total

php bin/phpstan analyze --debug bug-10772.php
  1.26s user 0.14s system 98% cpu 1.423 total

ondrejmirtes
ondrejmirtes requested changes May 10, 2024
View reviewed changes
tests/PHPStan/Analyser/AnalyserIntegrationTest.php Outdated
}

$errors = $this->runAnalyse(__DIR__ . '/data/bug-10979.php');
$this->assertCount(80, $errors);
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes May 10, 2024

Choose a reason for hiding this comment

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

Can you please fix the example so that it's valid and does not report any errors? And make sure it still reproduces the performance problem.

I'm worrying that the number 80 is going to fluctuate and is going to fail quite often as we work on PHPStan.

Copy link
Copy Markdown
Contributor Author

@staabm staabm May 11, 2024

Choose a reason for hiding this comment

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

we now have a zero errors repro

@ondrejmirtes ondrejmirtes merged commit 0463255 into phpstan:1.11.x May 11, 2024
@ondrejmirtes
Copy link
Copy Markdown
Member

ondrejmirtes commented May 11, 2024

Thank you!

@staabm staabm deleted the fix2 branch May 11, 2024 06:03
@staabm staabm mentioned this pull request May 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ondrejmirtes ondrejmirtes ondrejmirtes requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Larave11 makes parsing large Enum files considerably slower.

3 participants

@staabm @phpstan-bot @ondrejmirtes