Cache PHPStorm stubs - #4930
Conversation
| $stubsVersion = ComposerHelper::getPhpStormStubsVersion(); | ||
| $cacheKey = sprintf('phpstorm-stubs-%s', $stubsVersion); | ||
| $variableCacheKey = sprintf('v1-%s-%s', ComposerHelper::getBetterReflectionVersion(), $this->phpVersion->getVersionString()); |
There was a problem hiding this comment.
not sure whether this keys are correct this way.
whats the difference between $cacheKey and $variableCacheKey?
|
This pull request has been marked as ready for review. |
ondrejmirtes
left a comment
ondrejmirtes
left a comment
There was a problem hiding this comment.
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.
| continue; | ||
| } | ||
|
|
||
| $restored['constants'][$constantName] = ReflectionConstant::importFromCache($reflector, $cachedReflection); |
There was a problem hiding this comment.
ReflectionConstant/ReflectionParameter->importFromCache() is expensive.
it is dominated by PHPParser overhead
looks like the parser instantiation happens multiple times
There was a problem hiding this comment.
|
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; |
| } | ||
| } | ||
|
|
||
| $this->cache->save($this->cacheKey, $variableCacheKey, $exported); |
There was a problem hiding this comment.
Why it tries to have a single cache for everything? It could have a separate cache for each symbol.
|
at the current state I cannot see any perf improvement anymore. hopefully it gets better after ondrejmirtes/BetterReflection#41 is merged |
|
I think we have the same thing with turbo, so no need for the PR anymre |
4 participants
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)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.