Skip to content

IntersectionType: Prevent duplicate work in isOffsetAccessible()#5243

Merged
staabm merged 1 commit intophpstan:2.1.xfrom
staabm:int
Mar 18, 2026
Merged

IntersectionType: Prevent duplicate work in isOffsetAccessible()#5243
staabm merged 1 commit intophpstan:2.1.xfrom
staabm:int

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Mar 18, 2026

~29% faster in phpstan/phpstan#14319

grafik

after this change we are not yet fast, but its a low hanging fruit along the way

@staabm staabm requested a review from VincentLanglet March 18, 2026 06:46
@staabm staabm merged commit eb68e9a into phpstan:2.1.x Mar 18, 2026
650 of 652 checks passed
@staabm staabm deleted the int branch March 18, 2026 07:55
@VincentLanglet
Copy link
Copy Markdown
Contributor

I wonder @staabm

  • If this strategy should be done for some other methods of the IntersectionType ? (We could easily do all the TrinaryLogic methods with a refactor)
  • Why it's only memoized on IntersectionType (should some other type memoize too ? Either just UnionType which seems similar to Intersection or even all the types....)
  • Should we have a general memoize strategy with a trait ?

WDYT ?

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Mar 18, 2026

this small fix was guided by a blackfire profile which showed this slow path.
I usually add this caching only after we have evidence - and a real world case - that it is slow.

I did not change anything else, because this minimal fix was enough to get the case at hand faster.
IMO adding caching without the proof of a slow path makes things more complex, and might lead to copy/paste of caching into even more layers without a need for it.

profile before this FIX was

grafik

phpstan-bot pushed a commit to phpstan-bot/phpstan-src that referenced this pull request Apr 7, 2026
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.

2 participants