Skip to content

perf: Cache QueryBuilder class classification - #778

Open
realFlowControl wants to merge 2 commits into
phpstan:2.0.xfrom
realFlowControl:optimize/querybuilder-classification-cache
Open

perf: Cache QueryBuilder class classification#778
realFlowControl wants to merge 2 commits into
phpstan:2.0.xfrom
realFlowControl:optimize/querybuilder-classification-cache

Conversation

@realFlowControl

Copy link
Copy Markdown
Contributor

Cache whether each stable receiver class name is a QueryBuilder instead of repeating ClassReflection::is() for every call on that class.

Receiver classification is argument- and scope-independent, and the measured resolver performs more than 325,000 receiver reflection passes. Caching only this boolean avoids repeated hierarchy checks without caching scope-dependent method resolution.

This reduced median elapsed time from 28.45 s to 28.22 s (-0.8%) and aggregate user+sys CPU from 168.90 s to 167.37 s (-0.9%).

Benchmark run locally using PHPStan 2.2.x and PHPStan-Doctrine 2.0.x on MacOS with PHP 8.5.7

realFlowControl and others added 2 commits July 22, 2026 18:09
Cache whether each stable receiver class name is a QueryBuilder instead of repeating ClassReflection::is() for every call on that class.

Receiver classification is argument- and scope-independent, and the measured resolver performs more than 325,000 receiver reflection passes. Caching only this boolean avoids repeated hierarchy checks without caching scope-dependent method resolution.

Benchmark caveat: the recorded five-run Phase 5 candidate included this cache plus the separately committed cheap-check reorder; the cache was not measured alone. Together they reduced median elapsed time from 28.45 s to 27.57 s (-3.1%) and aggregate user+sys CPU from 168.90 s to 164.70 s (-2.5%). All five follow-up optimizations combined reduced elapsed time by 8.7% and CPU by 5.4% across 20 interleaved runs.
@realFlowControl
realFlowControl marked this pull request as ready for review July 23, 2026 08:58

@staabm staabm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

while the change is tiny, I wonder whether such a small benefit is worth caching?

how much overhead does this extension cost the static analysis run on your project?
maybe its worth building a list of method names using a collector, so we could prevent the is call alltogehter for most calls based on method name (whether thats worth the effort depends on hoch much impact this could have)?

if ($callerClassReflection->is(QueryBuilder::class)) {
$className = $callerClassReflection->getName();
if (($this->queryBuilderClasses[$className] ??= $callerClassReflection->is(QueryBuilder::class))) {
return null; // covered by QueryBuilderMethodDynamicReturnTypeExtension

@staabm staabm Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OT: this comment made me curious. looking into QueryBuilderMethodDynamicReturnTypeExtension and
made me realize that we have a config arround to declare what the QueryBuilder class is, since 4d630c6

@staabm
staabm requested a review from VincentLanglet July 23, 2026 09:19
@VincentLanglet

Copy link
Copy Markdown
Contributor

while the change is tiny, I wonder whether such a small benefit is worth caching?

I agree, I dunno what's the strategy about such perf improvement

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