turbo: One canonical instance per type (3% faster) - #6261
Open
AJenbo wants to merge 2 commits into
Open
Conversation
AJenbo
marked this pull request as draft
August 24, 2026 21:49
This is ported over from PHPantom and how it handles types internally. The benefits aren't as big as I had hoped since part of it was already being performed in the majority of cases, but it still provided a very real 3% improvement to turbo's performance. Keep one canonical instance per distinct type value, keyed by the same 128-bit structural hash the memo already computes. Operations that reach the same value by different routes then hand back the same object, so the identity checks already on the hot paths — pt_types_identical_or_equal() on scope merges, `$a === $b` in TypeCombinator::doUnion() — decide comparisons that used to recurse into two graphs. Entries are borrowed exactly like the memo's: a weak map whose dtor releases the slot, so a canonical instance is retained only as long as some live scope holds it anyway. The open-addressing table is now a SlotTable shared by the memo and the intern table. Worth -3.1% user CPU on serial self-analysis (81.8 -> 79.2 s, three interleaved rotated rounds). Instrumented over src/Type + src/Analyser: 793k TypeCombinator calls, 184k memo misses, and 110,874 of those misses (60%) recompute a value that already exists — it reduces the lifetime type instance count by 60%, though since they're short-lived there's no real memory benefit to this; only 73k distinct values are ever canonical. Every substitution was verified equals()-true and describe()-identical. Peak RSS is unchanged — the deduplicated results are short-lived, and peak RSS is set by reflection and parser state instead. Interning is only sound because doUnion() now returns the same type for equal array operands as for identical ones; before that fix, making the operands identical changed inference at tests/PHPStan/Rules/Variables/data/bug-8113.php:47 and analysis output was no longer identical with the extension on and off. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AJenbo
marked this pull request as ready for review
August 24, 2026 21:59
Collaborator
|
This pull request has been marked as ready for review. |
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
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.
This is ported over from PHPantom and how it handles types internally. The benefits aren't as big as I had hoped since part of it was already being performed in the majority of cases but it still provided a very real 3% improvement to turbo's performance.
It reduces life time type instances by 60% but since they are short lived there's no real memory benefit to this.
Entries are borrowed exactly like the memo's: a weak map whose dtor releases the slot, so a canonical instance is retained only as long as some live scope holds it anyway. The open-addressing table is now a SlotTable shared by the memo and the intern table.
Interning is only sound because doUnion() now returns the same type for equal array operands as for identical ones; before that fix, making the operands identical changed inference at tests/PHPStan/Rules/Variables/data/bug-8113.php:47 and analysis output was no longer identical with the extension on and off.