Skip to content

Conversation

@staabm
Copy link
Contributor

@staabm staabm commented Feb 10, 2026

as requested in #4823 (comment)

@staabm staabm marked this pull request as ready for review February 10, 2026 10:45
Copy link
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.

Also the performance should be observed with special care. Does anything gets slower because of this?

@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@staabm
Copy link
Contributor Author

staabm commented Feb 10, 2026

Does anything gets slower because of this?

before PR:

➜  phpstan-src git:(2.1.x) ✗ hyperfine 'vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php' --runs=5 
Benchmark 1: vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php
  Time (mean ± σ):     17.367 s ±  0.437 s    [User: 15.445 s, System: 0.599 s]
  Range (min … max):   16.896 s … 18.083 s    5 runs

with this PR 8c8b65a:

➜  phpstan-src git:(dis-arr) ✗ hyperfine 'vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php' --runs=5 
Benchmark 1: vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php
  Time (mean ± σ):     17.080 s ±  0.285 s    [User: 14.936 s, System: 0.631 s]
  Range (min … max):   16.744 s … 17.378 s    5 runs

I think its within the margin of error (because my company laptop has varying performance because of anti-virus stuff)

@ondrejmirtes
Copy link
Member

I'm still worried this can break stuff. I imagine that some $this->degradeToGeneralArray = true; assignments should still be allowed to happen but some should be guarded with !$this->disableArrayDegradation. Instead of your current way of changing conditions into if (!$this->degradeToGeneralArray || $this->disableArrayDegradation) {.

One example: we want $this->degradeToGeneralArray = true; to happen and be respected at the bottom of setOffsetValueType method. Because when someone assigns general IntegerType, it doesn't make sense to keep the array shape.

@staabm
Copy link
Contributor Author

staabm commented Feb 10, 2026

ok, how do you feel about it now?

with this PR 71303cc:

➜  phpstan-src git:(dis-arr) ✗ hyperfine 'vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php' --runs=5 
Benchmark 1: vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php
  Time (mean ± σ):     16.712 s ±  0.157 s    [User: 15.144 s, System: 0.609 s]
  Range (min … max):   16.459 s … 16.848 s    5 runs

@ondrejmirtes
Copy link
Member

This makes sense to me now :) Thank you!

@ondrejmirtes ondrejmirtes merged commit 13a2f02 into phpstan:2.1.x Feb 10, 2026
634 of 639 checks passed
@staabm staabm deleted the dis-arr branch February 10, 2026 13:08
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.

3 participants