Repository navigation
Store uses of a file once in the file type cache and in the result cache's exported PHPDoc nodes - #6577
Closed
phpstan-bot wants to merge 1 commit into
Closed
Store uses of a file once in the file type cache and in the result cache's exported PHPDoc nodes#6577phpstan-bot wants to merge 1 commit into
phpstan-bot wants to merge 1 commit into
Conversation
…che's exported PHPDoc nodes - serialize() keeps shared objects shared but writes every array by value, so each IntermediaryNameScope in a cached name scope map carried its own copy of the file's uses/constUses. The cache entry and the peak memory of loading it grew with the number of imports times the number of PHPDocs, so a warm run could use more memory than a cold one. - Add IntermediaryNameScope::dehydrateMap()/hydrateMap(): FileTypeMapper now stores every distinct array (uses, constUses, template tags, type aliases) and every scope once and refers to them by index. This replaces the interning after load (IntermediaryNameScope::intern()), because the peak is already reached while unserializing. File type cache key bumped to v7. - Analogous case in the result cache: every ExportedPhpDocNode held its own uses/constUses arrays, so exportedNodes grew the same way. ExportedPhpDocNode now holds the ExportedNameScope shared by all PHPDocs of a file, which serialize() writes once per file. ExportedNameScope::decode() reuses the previously decoded equal scope, so nodes sent by workers as JSON share it again in the main process. Result cache version bumped. - Other Cache::save() callers (reflection exports in the source locators) don't store per-member copies of file-level arrays, so they don't have this problem.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A file with many
usestatements and many PHPDoc blocks made the file type cache entry huge, because every name scope stored its own copy of the file's import map. Loading such an entry could use more memory than building it from scratch. This PR stores each import map once per cache entry. The same problem existed in the result cache's exported PHPDoc nodes, and this PR fixes that too.On a reproducer shaped like the issue's (572 imports, 1150 methods with
@returnPHPDocs):--debug -vvv)resultCache.phpChanges
src/Analyser/IntermediaryNameScope.php: newdehydrateMap()/hydrateMap(). The dehydrated form lists every distinct array (uses, constUses, template tags, type aliases) and every scope, including parents, once. Scopes refer to them by index. Removedintern(), which only rebuilt the sharing after the full copy was already unserialized.src/Type/FileTypeMapper.php: saves and loads the dehydrated form. The cache key is bumped tov7.src/Dependency/ExportedNode/ExportedPhpDocNode.php: holds anExportedNameScopeinstead of separate namespace/uses/constUses fields. The JSON format is unchanged.src/Dependency/ExportedNameScope.php: newequals()anddecode().decode()reuses the previously decoded scope when it is equal, so PHPDoc nodes of one file that a worker sends as JSON share one scope again in the main process.src/Dependency/ExportedNodeResolver.php: passes the shared name scope.src/Analyser/ResultCache/ResultCacheManager.php:CACHE_VERSIONis bumped because the serialized shape ofExportedPhpDocNodechanged.Cache::save()callers (reflection exports in the source locators) don't store per-member copies of file-level arrays, so they need no change.Root cause
The file cache and the result cache both use
serialize(). It keeps objects that are shared within one payload shared, but it writes arrays by value. Two places kept the same file-level arrays in many objects:IntermediaryNameScope::$uses/$constUses(and template/type-alias arrays), one per PHPDoc'd member, in the file type cache.ExportedPhpDocNode::$uses/$constUses, one per exported PHPDoc, in the result cache'sexportedNodes.So both payloads grew with the number of imports times the number of PHPDocs. The fix stores each array once: by index in the file type cache, and through a shared object in the result cache.
Test
FileTypeMapperTest::testNameScopeMapIsStoredInCacheWithSharedValuesuses a cache storage that serializes. It checks that the stored entry fordata/bug-15304.phpcontains each imported name once (it was 7 before). It also checks that a secondFileTypeMapperreading that entry resolves aliased@returntypes and class/method template types correctly.ExportedNodeResolverTest::testUsesAreStoredOncePerFilechecks that the serialized exported nodes of the same file contain each import once (it was 6 before). It also runs a JSON encode/decode round trip, as the parallel worker path does, and checks the decoded nodes are equal and still share their name scope.Fixes phpstan/phpstan#15304