Skip to content

Comments

Cache PHPStorm stubs#4930

Draft
staabm wants to merge 7 commits intophpstan:2.1.xfrom
staabm:cache-stubs
Draft

Cache PHPStorm stubs#4930
staabm wants to merge 7 commits intophpstan:2.1.xfrom
staabm:cache-stubs

Conversation

@staabm
Copy link
Contributor

@staabm staabm commented Feb 14, 2026

refs phpstan/phpstan#14072 (comment)

requires ondrejmirtes/BetterReflection#40


with caching we see a ~5-6% perf improvement when result-cache is warmed. (single process with --debug)

grafik

I guess the caching is even more effective in parallel runs because it removes PHP-Parser::parse() of PHPStorm stubs on every worker-process creation.

Comment on lines 35 to 37
$stubsVersion = ComposerHelper::getPhpStormStubsVersion();
$cacheKey = sprintf('phpstorm-stubs-%s', $stubsVersion);
$variableCacheKey = sprintf('v1-%s-%s', ComposerHelper::getBetterReflectionVersion(), $this->phpVersion->getVersionString());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure whether this keys are correct this way.
whats the difference between $cacheKey and $variableCacheKey?

@staabm staabm requested a review from ondrejmirtes February 14, 2026 14:55
@staabm staabm marked this pull request as ready for review February 14, 2026 17:58
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

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.

Personally I'd rather cache the same things the current locators cache. If you cache the "stubs", they are just PHP code that still needs to be parsed by the locator. So rather I'd have a caching locator around PhpInternalSourceLocator that would cache reflection objects which already have importFromCache/exportToCache.

@staabm staabm marked this pull request as draft February 21, 2026 16:57
continue;
}

$restored['constants'][$constantName] = ReflectionConstant::importFromCache($reflector, $cachedReflection);
Copy link
Contributor Author

@staabm staabm Feb 22, 2026

Choose a reason for hiding this comment

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

ReflectionConstant/ReflectionParameter->importFromCache() is expensive.
it is dominated by PHPParser overhead

Image

looks like the parser instantiation happens multiple times

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ondrejmirtes
Copy link
Member

For example one downside of this approach is that it loads into memory symbols it might not need in the current run.

$installed = self::getInstalled();
$rootPackage = $installed['versions']['jetbrains/phpstorm-stubs'] ?? null;
if ($rootPackage === null) {
return self::$phpstomStubsVersion = self::UNKNOWN_VERSION;
Copy link
Member

Choose a reason for hiding this comment

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

typo phpstom

}
}

$this->cache->save($this->cacheKey, $variableCacheKey, $exported);
Copy link
Member

Choose a reason for hiding this comment

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

Why it tries to have a single cache for everything? It could have a separate cache for each symbol.

@staabm
Copy link
Contributor Author

staabm commented Feb 22, 2026

at the current state I cannot see any perf improvement anymore.

hopefully it gets better after ondrejmirtes/BetterReflection#41 is merged

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