Repository navigation
Reuse parsed PHPDocs of equal strings - #6732
SanderMuller wants to merge 1 commit into
Conversation
| private const PARSED_PHPDOC_CACHE_LIMIT = 1024; | ||
|
|
||
| /** @var array<string, PhpDocNode> least recently used first */ | ||
| private array $parsedPhpDocs = []; |
There was a problem hiding this comment.
Really nice find, but please use LruCache here, and also drop the testLeastRecentlyUsedNodeIsEvicted test.
There was a problem hiding this comment.
Also the limit should be configurable with similar to keys under the cache: section.
There was a problem hiding this comment.
Done. It uses LruCache now, the limit is cache.parsedPhpDocCountMax (1024 by default), and I removed testLeastRecentlyUsedNodeIsEvicted.
|
would be great to rebase and re-measure this (memory and cpu time); and get it over the finish line |
|
Sorry, saw these comments and then lost track of them, looking into it |
FileTypeMapper, InvalidPhpDocTagValueRule and InvalidPHPStanDocTagRule parse the same PHPDoc strings, and the two rules parse each one directly after the other. PhpDocStringResolver now keeps the most recently parsed nodes in an LruCache, and the two rules use it instead of their own lexer and parser. The new cache.parsedPhpDocCountMax parameter sets how many nodes it keeps, 1024 by default. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
70e8d11 to
fbcde4b
Compare
|
Rebased and re-measured. The numbers are in the description. |
|
judging by the numbers in the descriptions it seems this does not really improve memory or cpu time. I will leave the decision wether its worth it to @ondrejmirtes |
InvalidPhpDocTagValueRuleandInvalidPHPStanDocTagRuleeach parse a node's doc comment with their own lexer and parser, one right after the other.FileTypeMapperparses many of the same strings throughPhpDocStringResolver. This PR keeps the most recently parsed nodes in anLruCacheinPhpDocStringResolver, and the two rules now use it. The newcache.parsedPhpDocCountMaxparameter sets how many nodes it keeps, 1024 by default.Nothing in
src/changes a parsedPhpDocNode, so equal strings can share one node.PhpDocEditorparses its own node and clones it before it edits.resolve()also consumesTOKEN_END, which the rules did not do. That cannot throw here, because a doc comment from php-parser ends at*/.Cold runs on 2.3.x at acd0511, one process (
--debug), Turbo off, 3 interleaved runs per arm, PHP 8.5 on macOS arm64. The output was identical between the arms in every run.src/, level 6: instructionssrc/, level 6: CPU timesrc/, level 6:memory_get_peak_usage()memory_get_peak_usage()The CPU time difference is within the noise. On the application, the cache keeps about 4.6 MiB more memory at peak. An earlier version of this PR, with the same 1024 limit, saved 2.6% of instructions on
src/and 0.6% on the application with Turbo on. I did not measure Turbo on again for this version.PhpDocStringResolverTest::testEqualStringsShareOneNodechecks that equal strings give the same node. It fails when the resolver does not store the node.🤖 Generated with Claude Code