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

Slow analysis when working with multiple big const arrays #8215

Closed
dpeukert opened this issue Oct 26, 2022 · 6 comments · Fixed by phpstan/phpstan-src#2116
Closed

Slow analysis when working with multiple big const arrays #8215

dpeukert opened this issue Oct 26, 2022 · 6 comments · Fixed by phpstan/phpstan-src#2116

Comments

@dpeukert
Copy link

dpeukert commented Oct 26, 2022

Bug report

Getting a value from a big (6500 values in the snippet) const array and using it to access a different similarly sized const array causes PHPStan to take about 70 seconds to analyze the file. I'm fairly sure the affected file in our codebase used to take no significant time to analyse before, but there were some changes to it around the time we updated PHPStan, so I'll check with some older versions of PHPStan to see if it is a regression and update the issue.

EDIT: Doesn't seem like a regression, see times below.

parameters:
    level: 8
./vendor/bin/phpstan analyse -c phpstan.neon --debug -vvv test.php

# PHPStan v1.8.11
# --- consumed 92 MB, total 128 MB, took 67.74 s

# PHPStan v1.8.2
# --- consumed 74 MB, total 132 MB, took 209.80 s

# PHPStan v1.4.8
# --- consumed 74 MB, total 130 MB (time reports 110.67 s)

Code snippet that reproduces the problem

The snippet causes the https://api.phpstan.org/analyse endpoint to return a 502, presumably because of the size, so here's a gist: https://gist.github.com/dpeukert/eadf816dc5cde760c1aa295fc065815e

Expected output

I would expect the analysis to be quicker.

Did PHPStan help you today? Did it make you happy in any way?

PHPStan rocks!

@mergeable
Copy link

mergeable bot commented Oct 26, 2022

This bug report is missing a link to reproduction at phpstan.org/try.

It will most likely be closed after manual review.

@staabm
Copy link
Contributor

staabm commented Dec 15, 2022

@ondrejmirtes what do you think about counting the number of elements in a constant array with a NodeVisitor and turn it into a OversizedArrayType instead of building all constant values into objects at first and let the type-analysis later on turn it into a OversizedArrayType?

doing so early and preventing the building up of all these Constant* object, which finally degrade seems like overhead, when we know a constant array has a certain number of elements..

@ondrejmirtes
Copy link
Member

@staabm You don't need to ask me, you can just try out the solution and see if it works. Myself I tried several things in the last few days and none of them worked.

Ideally I'd like a solution that solves this problem for more than just one example. For example this one https://github.com/phpstan/phpstan-src/blob/aa7b1eabd197ef6b7fb2c8eebbb7876aebc0ba6a/tests/PHPStan/Analyser/data/bug-8146.php is really hard. 95% of time is spent in TypeCombinator::union() because PHPStan tries to collapse the arrays in values of the first-level array.

@staabm
Copy link
Contributor

staabm commented Jan 3, 2023

@dpeukert this should be pretty fast now starting with 1.9.5

@dpeukert
Copy link
Author

dpeukert commented Jan 4, 2023

@staabm Thanks for the fix, can confirm that analysis of both the test case and our codebase sped up massively:

./vendor/bin/phpstan analyse -c phpstan.neon --debug -vvv test.php

# PHPStan v1.9.6
# --- consumed 88 MB, total 126 MB, took 0.51 s

@github-actions
Copy link

github-actions bot commented Feb 5, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants