Skip to content

Fix slow analysis on return with a big OR condition#5053

Closed
staabm wants to merge 6 commits intophpstan:2.1.xfrom
staabm:pr2
Closed

Fix slow analysis on return with a big OR condition#5053
staabm wants to merge 6 commits intophpstan:2.1.xfrom
staabm:pr2

Conversation

@staabm
Copy link
Contributor

@staabm staabm commented Feb 27, 2026

Closes phpstan/phpstan#14207

Regression test before PR:

➜  phpstan-src git:(2.1.x) ✗ time vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php  
PHPUnit 11.5.52 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.30
Configuration: /Users/m.staab/dvl/phpstan-src/phpunit.xml
Random Seed:   1772116653

...............................................................  63 / 212 ( 29%)
............................................................... 126 / 212 ( 59%)
............................................................... 189 / 212 ( 89%)
....S..................                                         212 / 212 (100%)

Time: 00:27.512, Memory: 334.50 MB

After PR (equal fast as AnalyserIntegrationTest on 2.1.x):

➜  phpstan-src git:(bug-14207-2) ✗ time vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php  
PHPUnit 11.5.52 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.30
Configuration: /Users/m.staab/dvl/phpstan-src/phpunit.xml
Random Seed:   1772116718

...............................................................  63 / 211 ( 29%)
............................................................... 126 / 211 ( 59%)
......S........................................................ 189 / 211 ( 89%)
......................                                          211 / 211 (100%)

Time: 00:17.676, Memory: 326.50 MB

Wordpress phpstan analyse --debug src/wp-includes/html-api/class-wp-html-processor.php -v before PR:

Elapsed time: 8.61 seconds

after PR:

Elapsed time: 3.89 seconds

@phpstan-bot
Copy link
Collaborator

You've opened the pull request against the latest branch 2.2.x. PHPStan 2.2 is not going to be released for months. If your code is relevant on 2.1.x and you want it to be released sooner, please rebase your pull request and change its target to 2.1.x.

@staabm staabm changed the base branch from 2.2.x to 2.1.x February 27, 2026 08:23
@staabm staabm marked this pull request as ready for review February 27, 2026 08:37
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@staabm
Copy link
Contributor Author

staabm commented Feb 27, 2026

what I like about this fix

  • its ~60% faster than before
  • its a simple fix

what I don't like about this fix

  • the repro still in the 3-4 second range of analysis time for a single file (since the file has ~6600 lines of code, this might be acceptable though)

@staabm
Copy link
Contributor Author

staabm commented Feb 27, 2026

the issue-bot result seems outdated, as I get the same diff when I open the playground link and click "Show the latest result" (so the diff the bot reports with this PR, is already on 2.1.x)

@staabm staabm requested a review from ondrejmirtes February 27, 2026 09:17
continue;
}

if ($type instanceof UnionType && count($type->getTypes()) >= self::MAX_SURE_NOT_TYPES) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd limit the sureTypes the same way, just to be consistent and find possible issues with this approach

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so you mean we should generalize a sure-type when this limit is hit?
not beeing precise about what we not know has less impact on the overall analysis as generalizing what we know though.

Copy link
Member

Choose a reason for hiding this comment

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

I just meant to copy:

			if ($type instanceof UnionType && count($type->getTypes()) >= self::MAX_SURE_NOT_TYPES) { 
				continue;
			}

in the foreach ($this->sureTypes... above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok - did just that. I don't understand why so ;-)

Copy link
Member

Choose a reason for hiding this comment

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

The two arrays have the same importance. I feel like what we do to one we should do to the other.

If some tests start failing, it means the fix is wrong for both sureTypes and sureNotTypes.

@ondrejmirtes
Copy link
Member

Superseded by #5076

@staabm staabm deleted the pr2 branch February 27, 2026 17: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.

slow analysis on return with a big OR condition (wordpress)

3 participants