Skip to content

Commit c0c6b05

Browse files
KocalKocal
committed
Check a type alias used by several keys once for a missing value type
A type alias referenced by many keys of an array shape is one shared Type object. `MissingTypehintCheck::getIterableTypesWithMissingValueTypehint()` walked that object again for every key referring to it, and for every constant array it met it also ran a second full walk to drop its unsealed part. The array shape that symfony/ux-css generates for its `css()` Twig function has about 840 keys, 126 of them referring to a nested shape, and checking one method parameter against it took seconds. The callback only depends on the type it's given, so each object is now checked once. Later visits of the same object replay the descriptions added during the first visit, in the same order, so reported errors don't change, duplicates included. The inner walk that drops unsealed parts caches its result per object the same way. Objects are keyed with `spl_object_id()` and kept in the cache for the duration of the walk, so an id can't be reused in between. | Scenario | Before | After | |---|---|---| | New `nested-array-shape-alias.php` bench | 117.4-117.9 ms | 70.4-70.7 ms | | Real ux-css shape, 2 levels, 1 call, turbo extension loaded | 5.37-5.41 s | 4.41-4.43 s | The rest of the phpbench suite stays within its existing thresholds. The two scenarios with the largest difference, bug-7140 and bug-8503, were re-run 3 times with 15 iterations each, and the difference turned out to be noise (22.40-22.57 ms vs 22.41-22.49 ms, and 123.9-125.6 ms vs 122.9-127.1 ms). ``` XDEBUG_MODE=off tests/vendor/bin/phpbench run --variant=nested-array-shape-alias.php --iterations=10 --report=aggregate ```
1 parent 261a431 commit c0c6b05

2 files changed

Lines changed: 71 additions & 14 deletions

File tree

‎src/Rules/MissingTypehintCheck.php‎

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,16 @@
2424
use PHPStan\Type\ObjectType;
2525
use PHPStan\Type\Type;
2626
use PHPStan\Type\TypeTraverser;
27-
use PHPStan\Type\UnionType;
2827
use PHPStan\Type\VerbosityLevel;
2928
use Traversable;
3029
use function array_filter;
3130
use function array_keys;
3231
use function array_merge;
32+
use function array_slice;
3333
use function count;
3434
use function implode;
3535
use function in_array;
36+
use function spl_object_id;
3637
use function sprintf;
3738
use function strtolower;
3839

@@ -73,7 +74,8 @@ public function __construct(
7374
public function getIterableTypesWithMissingValueTypehint(Type $type): array
7475
{
7576
$descriptions = [];
76-
TypeTraverser::map($type, function (Type $type, callable $traverse) use (&$descriptions): Type {
77+
$sealed = [];
78+
TypeTraverser::map($type, self::oncePerType($descriptions, function (Type $type, callable $traverse) use (&$descriptions, &$sealed): Type {
7779
if ($type instanceof TemplateType) {
7880
return $type;
7981
}
@@ -108,19 +110,16 @@ public function getIterableTypesWithMissingValueTypehint(Type $type): array
108110
}
109111
if ($type->isIterable()->yes()) {
110112
if ($type->isConstantArray()->yes()) {
111-
$type = TypeTraverser::map($type, static function (Type $type, callable $traverse) {
112-
if ($type instanceof UnionType || $type instanceof IntersectionType) {
113-
return $traverse($type);
113+
$type = TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$sealed) {
114+
$id = spl_object_id($type);
115+
if (!isset($sealed[$id])) {
116+
$sealedType = $type instanceof ConstantArrayType && $type->getUnsealedTypes() !== null
117+
? $type->dropUnsealedTypes()
118+
: $type;
119+
$sealed[$id] = [$type, $traverse($sealedType)];
114120
}
115121

116-
if ($type instanceof ConstantArrayType) {
117-
$unsealed = $type->getUnsealedTypes();
118-
if ($unsealed !== null) {
119-
return $traverse($type->dropUnsealedTypes());
120-
}
121-
}
122-
123-
return $traverse($type);
122+
return $sealed[$id][1];
124123
});
125124
}
126125
$iterableValue = $type->getIterableValueType();
@@ -136,11 +135,41 @@ public function getIterableTypesWithMissingValueTypehint(Type $type): array
136135
}
137136
}
138137
return $traverse($type);
139-
});
138+
}));
140139

141140
return $descriptions;
142141
}
143142

143+
/**
144+
* A type alias used in several places is one shared object: check it once, then replay the descriptions it added.
145+
*
146+
* @param string[] $descriptions
147+
* @param callable(Type, callable(Type): Type): Type $check
148+
* @return callable(Type, callable(Type): Type): Type
149+
*/
150+
private static function oncePerType(array &$descriptions, callable $check): callable
151+
{
152+
$checked = [];
153+
154+
return static function (Type $type, callable $traverse) use ($check, &$checked, &$descriptions): Type {
155+
$id = spl_object_id($type);
156+
if (isset($checked[$id])) {
157+
[, $result, $added] = $checked[$id];
158+
foreach ($added as $description) {
159+
$descriptions[] = $description;
160+
}
161+
162+
return $result;
163+
}
164+
165+
$count = count($descriptions);
166+
$result = $check($type, $traverse);
167+
$checked[$id] = [$type, $result, array_slice($descriptions, $count)];
168+
169+
return $result;
170+
};
171+
}
172+
144173
/**
145174
* @return array<int, array{string, string}>
146175
*/

‎tests/bench/data/nested-array-shape-alias.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace BenchNestedArrayShapeAlias;
4+
5+
/**
6+
* A type alias that many keys of an array shape refer to is one shared object. Checking the
7+
* parameter below for a missing iterable value type used to walk that object again from every
8+
* key that refers to it.
9+
*
10+
* @phpstan-type Value = 'a'|'b'|'c'|'d'|string
11+
* @phpstan-type Inner = array{p0?: Value, p1?: Value, p2?: Value, p3?: Value, p4?: Value, p5?: Value, p6?: Value, p7?: Value, p8?: Value, p9?: Value, p10?: Value, p11?: Value, p12?: Value, p13?: Value, p14?: Value, p15?: Value, p16?: Value, p17?: Value, p18?: Value, p19?: Value, p20?: Value, p21?: Value, p22?: Value, p23?: Value, p24?: Value, p25?: Value, p26?: Value, p27?: Value, p28?: Value, p29?: Value, p30?: Value, p31?: Value, p32?: Value, p33?: Value, p34?: Value, p35?: Value, p36?: Value, p37?: Value, p38?: Value, p39?: Value, p40?: Value, p41?: Value, p42?: Value, p43?: Value, p44?: Value, p45?: Value, p46?: Value, p47?: Value, p48?: Value, p49?: Value, p50?: Value, p51?: Value, p52?: Value, p53?: Value, p54?: Value, p55?: Value, p56?: Value, p57?: Value, p58?: Value, p59?: Value, p60?: Value, p61?: Value, p62?: Value, p63?: Value, p64?: Value, p65?: Value, p66?: Value, p67?: Value, p68?: Value, p69?: Value, p70?: Value, p71?: Value, p72?: Value, p73?: Value, p74?: Value, p75?: Value, p76?: Value, p77?: Value, p78?: Value, p79?: Value, p80?: Value, p81?: Value, p82?: Value, p83?: Value, p84?: Value, p85?: Value, p86?: Value, p87?: Value, p88?: Value, p89?: Value, p90?: Value, p91?: Value, p92?: Value, p93?: Value, p94?: Value, p95?: Value, p96?: Value, p97?: Value, p98?: Value, p99?: Value, p100?: Value, p101?: Value, p102?: Value, p103?: Value, p104?: Value, p105?: Value, p106?: Value, p107?: Value, p108?: Value, p109?: Value, p110?: Value, p111?: Value, p112?: Value, p113?: Value, p114?: Value, p115?: Value, p116?: Value, p117?: Value, p118?: Value, p119?: Value, p120?: Value, p121?: Value, p122?: Value, p123?: Value, p124?: Value, p125?: Value, p126?: Value, p127?: Value, p128?: Value, p129?: Value, p130?: Value, p131?: Value, p132?: Value, p133?: Value, p134?: Value, p135?: Value, p136?: Value, p137?: Value, p138?: Value, p139?: Value, p140?: Value, p141?: Value, p142?: Value, p143?: Value, p144?: Value, p145?: Value, p146?: Value, p147?: Value, p148?: Value, p149?: Value}
12+
* @phpstan-type Outer = array{p0?: Value, p1?: Value, p2?: Value, p3?: Value, p4?: Value, p5?: Value, p6?: Value, p7?: Value, p8?: Value, p9?: Value, p10?: Value, p11?: Value, p12?: Value, p13?: Value, p14?: Value, p15?: Value, p16?: Value, p17?: Value, p18?: Value, p19?: Value, p20?: Value, p21?: Value, p22?: Value, p23?: Value, p24?: Value, p25?: Value, p26?: Value, p27?: Value, p28?: Value, p29?: Value, p30?: Value, p31?: Value, p32?: Value, p33?: Value, p34?: Value, p35?: Value, p36?: Value, p37?: Value, p38?: Value, p39?: Value, p40?: Value, p41?: Value, p42?: Value, p43?: Value, p44?: Value, p45?: Value, p46?: Value, p47?: Value, p48?: Value, p49?: Value, p50?: Value, p51?: Value, p52?: Value, p53?: Value, p54?: Value, p55?: Value, p56?: Value, p57?: Value, p58?: Value, p59?: Value, p60?: Value, p61?: Value, p62?: Value, p63?: Value, p64?: Value, p65?: Value, p66?: Value, p67?: Value, p68?: Value, p69?: Value, p70?: Value, p71?: Value, p72?: Value, p73?: Value, p74?: Value, p75?: Value, p76?: Value, p77?: Value, p78?: Value, p79?: Value, p80?: Value, p81?: Value, p82?: Value, p83?: Value, p84?: Value, p85?: Value, p86?: Value, p87?: Value, p88?: Value, p89?: Value, p90?: Value, p91?: Value, p92?: Value, p93?: Value, p94?: Value, p95?: Value, p96?: Value, p97?: Value, p98?: Value, p99?: Value, p100?: Value, p101?: Value, p102?: Value, p103?: Value, p104?: Value, p105?: Value, p106?: Value, p107?: Value, p108?: Value, p109?: Value, p110?: Value, p111?: Value, p112?: Value, p113?: Value, p114?: Value, p115?: Value, p116?: Value, p117?: Value, p118?: Value, p119?: Value, p120?: Value, p121?: Value, p122?: Value, p123?: Value, p124?: Value, p125?: Value, p126?: Value, p127?: Value, p128?: Value, p129?: Value, p130?: Value, p131?: Value, p132?: Value, p133?: Value, p134?: Value, p135?: Value, p136?: Value, p137?: Value, p138?: Value, p139?: Value, p140?: Value, p141?: Value, p142?: Value, p143?: Value, p144?: Value, p145?: Value, p146?: Value, p147?: Value, p148?: Value, p149?: Value, c0?: Inner, c1?: Inner, c2?: Inner, c3?: Inner, c4?: Inner, c5?: Inner, c6?: Inner, c7?: Inner, c8?: Inner, c9?: Inner, c10?: Inner, c11?: Inner, c12?: Inner, c13?: Inner, c14?: Inner, c15?: Inner, c16?: Inner, c17?: Inner, c18?: Inner, c19?: Inner, c20?: Inner, c21?: Inner, c22?: Inner, c23?: Inner, c24?: Inner, c25?: Inner, c26?: Inner, c27?: Inner, c28?: Inner, c29?: Inner, c30?: Inner, c31?: Inner, c32?: Inner, c33?: Inner, c34?: Inner, c35?: Inner, c36?: Inner, c37?: Inner, c38?: Inner, c39?: Inner}
13+
*/
14+
final class Styles
15+
{
16+
17+
/** @param Outer|null $styles */
18+
public function css(?array $styles): string
19+
{
20+
return '';
21+
}
22+
23+
}
24+
25+
function render(Styles $styles): void
26+
{
27+
echo $styles->css(['p0' => 'a', 'p1' => 'b', 'c0' => ['p2' => 'c']]);
28+
}

0 commit comments

Comments
 (0)