Skip to content

Commit 5ee84cd

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Drop the per-holder contained-node-keys invalidation index
With the compositional-key shortcut clearing virtual-node keys, only 308k invalidation checks reach the containment step on a src/Analyser+src/Rules run (down from 1.16M) - the population where the index was measured worthless on 2.2.x (phpstan-src PR #6152, 12 interleaved pairs, dead neutral). ExpressionTypeHolder, the ScopeOps containment (containsExpressionToInvalidate) and their native twins return to the 2.2.x shapes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
1 parent b52c3ca commit 5ee84cd

8 files changed

Lines changed: 75 additions & 366 deletions

File tree

‎src/Analyser/ExpressionTypeHolder.php‎

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2,33 +2,18 @@
22

33
namespace PHPStan\Analyser;
44

5-
use PhpParser\Node;
65
use PhpParser\Node\Expr;
76
use PHPStan\TrinaryLogic;
87
use PHPStan\Turbo\ReferencedByTurboExtension;
98
use PHPStan\Turbo\ShadowedByTurboExtension;
109
use PHPStan\Type\Type;
1110
use PHPStan\Type\TypeCombinator;
12-
use function array_pop;
13-
use function get_class;
14-
use function is_array;
1511

1612
#[ShadowedByTurboExtension(turboClass: 'PHPStanTurbo\ExpressionTypeHolder', implementation: __DIR__ . '/../../turbo-ext/src/ExpressionTypeHolder.cpp')]
1713
#[ReferencedByTurboExtension(key: 'expressionTypeHolder')]
1814
final class ExpressionTypeHolder
1915
{
2016

21-
/**
22-
* The node key of every sub-expression, keyed to the classes it appears
23-
* as - what MutatingScope::shouldInvalidateExpression()'s AST scan
24-
* established per invalidation. Holders are shared across scope copies,
25-
* so the one-time subtree scan amortizes over the many invalidation
26-
* checks against the same holder.
27-
*
28-
* @var array<string, array<class-string<Expr>, true>>|null
29-
*/
30-
private ?array $containedNodeKeys = null;
31-
3217
public function __construct(
3318
private readonly Expr $expr,
3419
private readonly Type $type,
@@ -37,42 +22,6 @@ public function __construct(
3722
{
3823
}
3924

40-
/**
41-
* @param callable(Expr): string $keyBuilder
42-
* @return array<string, array<class-string<Expr>, true>>
43-
*/
44-
public function getContainedNodeKeys(callable $keyBuilder): array
45-
{
46-
if ($this->containedNodeKeys !== null) {
47-
return $this->containedNodeKeys;
48-
}
49-
50-
$keys = [];
51-
$stack = [$this->expr];
52-
while ($stack !== []) {
53-
$node = array_pop($stack);
54-
if ($node instanceof Expr) {
55-
$keys[$keyBuilder($node)][get_class($node)] = true;
56-
}
57-
foreach ($node->getSubNodeNames() as $subNodeName) {
58-
$subNode = $node->$subNodeName;
59-
if ($subNode instanceof Node) {
60-
$stack[] = $subNode;
61-
} elseif (is_array($subNode)) {
62-
foreach ($subNode as $subNodeItem) {
63-
if (!($subNodeItem instanceof Node)) {
64-
continue;
65-
}
66-
67-
$stack[] = $subNodeItem;
68-
}
69-
}
70-
}
71-
}
72-
73-
return $this->containedNodeKeys = $keys;
74-
}
75-
7625
public static function createYes(Expr $expr, Type $type): self
7726
{
7827
return new self($expr, $type, TrinaryLogic::createYes());

‎src/Analyser/MutatingScope.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4390,7 +4390,7 @@ private function generalizeVariableTypeHolders(
43904390
$newVariableTypeHolders = [];
43914391
foreach ($variableTypeHolders as $variableExprString => $variableTypeHolder) {
43924392
foreach ($generalizedExpressions as $generalizedExprString => $generalizedExpr) {
4393-
if (!ScopeOps::shouldInvalidateExpression($this, $this->exprPrinter, $generalizedExprString, $generalizedExpr, $variableTypeHolder, $variableExprString)) {
4393+
if (!ScopeOps::shouldInvalidateExpression($this, $this->exprPrinter, $generalizedExprString, $generalizedExpr, $variableTypeHolder->getExpr(), $variableExprString)) {
43944394
continue;
43954395
}
43964396

‎src/Analyser/ScopeOps.php‎

Lines changed: 60 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use function count;
3030
use function get_class;
3131
use function in_array;
32+
use function is_array;
3233
use function is_string;
3334
use function str_contains;
3435
use function strlen;
@@ -651,15 +652,15 @@ public static function invalidateExpressionEntries(
651652
&& !str_contains($exprStringToInvalidate, '/*');
652653

653654
foreach ($expressionTypes as $exprString => $exprTypeHolder) {
654-
$exprString = (string) $exprString;
655+
$exprString = (string) $exprString; // @phpstan-ignore cast.useless
655656
if (
656657
$canUseKeyPrefilter
657658
&& !str_contains($exprString, $exprStringToInvalidate)
658659
&& !self::keyMayHideSubExpressions($exprString)
659660
) {
660661
continue;
661662
}
662-
if (!self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $exprTypeHolder, $exprString, $requireMoreCharacters, $invalidatingClass)) {
663+
if (!self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $exprTypeHolder->getExpr(), $exprString, $requireMoreCharacters, $invalidatingClass)) {
663664
continue;
664665
}
665666

@@ -673,13 +674,14 @@ public static function invalidateExpressionEntries(
673674
if (count($holders) === 0) {
674675
continue;
675676
}
677+
$conditionalExprString = (string) $conditionalExprString; // @phpstan-ignore cast.useless
676678
if (
677679
!$canUseKeyPrefilter
678-
|| str_contains((string) $conditionalExprString, $exprStringToInvalidate)
679-
|| self::keyMayHideSubExpressions((string) $conditionalExprString)
680+
|| str_contains($conditionalExprString, $exprStringToInvalidate)
681+
|| self::keyMayHideSubExpressions($conditionalExprString)
680682
) {
681683
$firstHolder = $holders[array_key_first($holders)]->getTypeHolder();
682-
if (self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $firstHolder, self::nodeKey($firstHolder->getExpr(), $exprPrinter), $requireMoreCharacters, $invalidatingClass)) {
684+
if (self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $firstHolder->getExpr(), self::nodeKey($firstHolder->getExpr(), $exprPrinter), $requireMoreCharacters, $invalidatingClass)) {
683685
$invalidated = true;
684686
continue;
685687
}
@@ -701,7 +703,7 @@ public static function invalidateExpressionEntries(
701703
$shouldKeep = true;
702704
$conditionalTypeHolders = $holder->getConditionExpressionTypeHolders();
703705
foreach ($conditionalTypeHolders as $conditionalTypeHolderExprString => $conditionalTypeHolder) {
704-
if (self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $conditionalTypeHolder, $conditionalTypeHolderExprString, invalidatingClass: $invalidatingClass)) {
706+
if (self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $conditionalTypeHolder->getExpr(), $conditionalTypeHolderExprString, invalidatingClass: $invalidatingClass)) {
705707
$invalidated = true;
706708
$shouldKeep = false;
707709
break;
@@ -728,14 +730,59 @@ public static function invalidateExpressionEntries(
728730
}
729731

730732
/**
731-
* Answers one (stored holder, invalidated expression) containment check
732-
* from the holder's contained-node-keys index; '$this' invalidations keep
733-
* the finder because they also match self/static/parent and the current
734-
* class name, whose resolution depends on the asking scope.
733+
* Depth-first pre-order search for the invalidated expression, replacing a
734+
* NodeFinder::findFirst() call - this runs for every (stored expression,
735+
* invalidated expression) pair whose keys pass the substring pre-filter,
736+
* so the traverser/visitor machinery overhead was significant.
737+
*
738+
* @param class-string<Expr> $expressionToInvalidateClass
739+
*/
740+
private static function containsExpressionToInvalidate(Scope $scope, ExprPrinter $exprPrinter, Node $node, string $expressionToInvalidateClass, string $exprStringToInvalidate): bool
741+
{
742+
if (
743+
$exprStringToInvalidate === '$this'
744+
&& $node instanceof Name
745+
&& (
746+
in_array($node->toLowerString(), ['self', 'static', 'parent'], true)
747+
|| ($scope->getClassReflection() !== null && $scope->getClassReflection()->is($scope->resolveName($node)))
748+
)
749+
) {
750+
return true;
751+
}
752+
753+
if (
754+
$node instanceof $expressionToInvalidateClass
755+
&& self::nodeKey($node, $exprPrinter) === $exprStringToInvalidate
756+
) {
757+
return true;
758+
}
759+
760+
foreach ($node->getSubNodeNames() as $subNodeName) {
761+
$subNode = $node->$subNodeName;
762+
if ($subNode instanceof Node) {
763+
if (self::containsExpressionToInvalidate($scope, $exprPrinter, $subNode, $expressionToInvalidateClass, $exprStringToInvalidate)) {
764+
return true;
765+
}
766+
} elseif (is_array($subNode)) {
767+
foreach ($subNode as $subNodeItem) {
768+
if (
769+
$subNodeItem instanceof Node
770+
&& self::containsExpressionToInvalidate($scope, $exprPrinter, $subNodeItem, $expressionToInvalidateClass, $exprStringToInvalidate)
771+
) {
772+
return true;
773+
}
774+
}
775+
}
776+
}
777+
778+
return false;
779+
}
780+
781+
/**
782+
* Mirrors the former MutatingScope::shouldInvalidateExpression().
735783
*/
736-
public static function shouldInvalidateExpression(MutatingScope $scope, ExprPrinter $exprPrinter, string $exprStringToInvalidate, Expr $exprToInvalidate, ExpressionTypeHolder $exprTypeHolder, string $exprString, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): bool
784+
public static function shouldInvalidateExpression(MutatingScope $scope, ExprPrinter $exprPrinter, string $exprStringToInvalidate, Expr $exprToInvalidate, Expr $expr, string $exprString, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): bool
737785
{
738-
$expr = $exprTypeHolder->getExpr();
739786
if (
740787
$expr instanceof IntertwinedVariableByReferenceWithExpr
741788
&& $exprToInvalidate instanceof Variable
@@ -778,34 +825,7 @@ public static function shouldInvalidateExpression(MutatingScope $scope, ExprPrin
778825
return false;
779826
}
780827

781-
if ($exprStringToInvalidate === '$this') {
782-
// '$this' also matches self/static/parent and the current class name -
783-
// name resolution depends on this scope, so the holder-cached key
784-
// index below cannot answer it
785-
$nodeFinder = new NodeFinder();
786-
$expressionToInvalidateClass = get_class($exprToInvalidate);
787-
$found = $nodeFinder->findFirst([$expr], static function (Node $node) use ($scope, $exprPrinter, $expressionToInvalidateClass, $exprStringToInvalidate): bool {
788-
if (
789-
$node instanceof Name
790-
&& (
791-
in_array($node->toLowerString(), ['self', 'static', 'parent'], true)
792-
|| ($scope->getClassReflection() !== null && $scope->getClassReflection()->is($scope->resolveName($node)))
793-
)
794-
) {
795-
return true;
796-
}
797-
798-
if (!$node instanceof $expressionToInvalidateClass) {
799-
return false;
800-
}
801-
802-
return self::nodeKey($node, $exprPrinter) === $exprStringToInvalidate;
803-
});
804-
805-
if ($found === null) {
806-
return false;
807-
}
808-
} elseif (!isset($exprTypeHolder->getContainedNodeKeys(static fn (Expr $node): string => self::nodeKey($node, $exprPrinter))[$exprStringToInvalidate][get_class($exprToInvalidate)])) {
828+
if (!self::containsExpressionToInvalidate($scope, $exprPrinter, $expr, get_class($exprToInvalidate), $exprStringToInvalidate)) {
809829
return false;
810830
}
811831

‎turbo-ext/src/ExpressionTypeHolder.cpp‎

Lines changed: 0 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -62,112 +62,6 @@ class ExpressionTypeHolder
6262
return zv::Val::adopt(result);
6363
}
6464

65-
/*
66-
* Mirrors ExpressionTypeHolder::getContainedNodeKeys(): lazily indexes
67-
* the node keys of every sub-expression of the holder's expr, keyed to
68-
* the classes it appears as. The walk replicates the twin's explicit
69-
* stack (LIFO pop order) so the cached array is key-order identical.
70-
* UNDEF = pending exception.
71-
*/
72-
zv::Val getContainedNodeKeys(zend_fcall_info *fci, zend_fcall_info_cache *fcc)
73-
{
74-
zv::ObjRef obj(self);
75-
zval *cached = obj.propAt(PT_ETH_PROP_CONTAINED_NODE_KEYS).raw();
76-
if (Z_TYPE_P(cached) == IS_ARRAY) {
77-
return zv::Val::copyOf(zv::Ref(cached));
78-
}
79-
80-
zend_class_entry *exprCe = pt_class(PT_CLASS_EXPR);
81-
zend_class_entry *nodeCe = pt_class(PT_CLASS_NODE);
82-
if (UNEXPECTED(exprCe == NULL || nodeCe == NULL)) {
83-
return zv::Val();
84-
}
85-
86-
zv::Arr keys = zv::Arr::create(8);
87-
88-
/* borrowed node pointers — pinned by the expr tree the holder owns */
89-
uint32_t cap = 32, top = 0;
90-
zend_object **stack = (zend_object **) emalloc(sizeof(*stack) * cap);
91-
stack[top++] = obj.propAt(PT_ETH_PROP_EXPR).deref().asObject();
92-
93-
bool failed = false;
94-
while (top > 0) {
95-
zend_object *node = stack[--top];
96-
97-
if (instanceof_function(node->ce, exprCe)) {
98-
/* $keys[$keyBuilder($node)][get_class($node)] = true */
99-
zval arg, retval;
100-
ZVAL_OBJ(&arg, node);
101-
fci->retval = &retval;
102-
fci->param_count = 1;
103-
fci->params = &arg;
104-
fci->named_params = NULL;
105-
if (UNEXPECTED(zend_call_function(fci, fcc) != SUCCESS || EG(exception))) {
106-
failed = true;
107-
break;
108-
}
109-
if (UNEXPECTED(Z_TYPE(retval) != IS_STRING)) {
110-
zval_ptr_dtor(&retval);
111-
zend_throw_error(NULL, "phpstan_turbo: getContainedNodeKeys() keyBuilder must return a string");
112-
failed = true;
113-
break;
114-
}
115-
zval *innerSlot = zend_symtable_find(keys.table(), Z_STR(retval));
116-
if (innerSlot == NULL) {
117-
zval innerZv;
118-
array_init(&innerZv);
119-
innerSlot = zend_symtable_update(keys.table(), Z_STR(retval), &innerZv);
120-
}
121-
zval trueZv;
122-
ZVAL_TRUE(&trueZv);
123-
zend_hash_update(Z_ARRVAL_P(innerSlot), node->ce->name, &trueZv);
124-
zval_ptr_dtor(&retval);
125-
}
126-
127-
pt_node_class_info *info = pt_node_class_info_for_object(node);
128-
if (info == NULL || !PT_HAS_SUBNODES(info)) {
129-
continue;
130-
}
131-
for (uint32_t i = 0; i < info->subnode_count; i++) {
132-
zval *val = OBJ_PROP(node, info->subnode_offsets[i]);
133-
ZVAL_DEREF(val);
134-
if (Z_TYPE_P(val) == IS_OBJECT) {
135-
if (instanceof_function(Z_OBJCE_P(val), nodeCe)) {
136-
if (UNEXPECTED(top == cap)) {
137-
cap *= 2;
138-
stack = (zend_object **) erealloc(stack, sizeof(*stack) * cap);
139-
}
140-
stack[top++] = Z_OBJ_P(val);
141-
}
142-
} else if (Z_TYPE_P(val) == IS_ARRAY) {
143-
zval *el;
144-
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(val), el) {
145-
zval *elDeref = el;
146-
ZVAL_DEREF(elDeref);
147-
if (Z_TYPE_P(elDeref) == IS_OBJECT && instanceof_function(Z_OBJCE_P(elDeref), nodeCe)) {
148-
if (UNEXPECTED(top == cap)) {
149-
cap *= 2;
150-
stack = (zend_object **) erealloc(stack, sizeof(*stack) * cap);
151-
}
152-
stack[top++] = Z_OBJ_P(elDeref);
153-
}
154-
} ZEND_HASH_FOREACH_END();
155-
}
156-
}
157-
}
158-
efree(stack);
159-
160-
if (UNEXPECTED(failed)) {
161-
return zv::Val();
162-
}
163-
164-
/* $this->containedNodeKeys = $keys (cache), then return it */
165-
zval *slot = obj.propAt(PT_ETH_PROP_CONTAINED_NODE_KEYS).raw();
166-
zval_ptr_dtor(slot);
167-
ZVAL_COPY(slot, keys.raw());
168-
return zv::Val(std::move(keys));
169-
}
170-
17165
zv::Val getExpr() const { return zv::Val::copyOf(zv::ObjRef(self).propAt(PT_ETH_PROP_EXPR)); }
17266
zv::Val getType() const { return zv::Val::copyOf(zv::ObjRef(self).propAt(PT_ETH_PROP_TYPE)); }
17367
zv::Val getCertainty() const { return zv::Val::copyOf(zv::ObjRef(self).propAt(PT_ETH_PROP_CERTAINTY)); }
@@ -195,7 +89,6 @@ void pt_register_expression_type_holder()
19589
cls.privateNullProperty("expr");
19690
cls.privateNullProperty("type");
19791
cls.privateNullProperty("certainty");
198-
cls.privateNullProperty("containedNodeKeys");
19992

20093
cls.method("__construct", reg::Public, 3, { reg::any("expr"), reg::any("type"), reg::obj("certainty", TRINARY_CLASS) }, [](INTERNAL_FUNCTION_PARAMETERS) {
20194
zval *expr, *type, *certainty;
@@ -261,19 +154,6 @@ void pt_register_expression_type_holder()
261154
result.intoReturnValue(return_value);
262155
});
263156

264-
cls.method("getContainedNodeKeys", reg::Public, 1, { reg::callableArg("keyBuilder") }, [](INTERNAL_FUNCTION_PARAMETERS) {
265-
zend_fcall_info fci;
266-
zend_fcall_info_cache fcc;
267-
ZEND_PARSE_PARAMETERS_START(1, 1)
268-
Z_PARAM_FUNC(fci, fcc)
269-
ZEND_PARSE_PARAMETERS_END();
270-
zv::Val result = ExpressionTypeHolder(ZEND_THIS).getContainedNodeKeys(&fci, &fcc);
271-
if (UNEXPECTED(result.isUndef())) {
272-
RETURN_THROWS();
273-
}
274-
result.intoReturnValue(return_value);
275-
});
276-
277157
cls.method("getExpr", reg::Public, 0, {}, [](INTERNAL_FUNCTION_PARAMETERS) {
278158
ZEND_PARSE_PARAMETERS_NONE();
279159
ExpressionTypeHolder(ZEND_THIS).getExpr().intoReturnValue(return_value);

0 commit comments

Comments
 (0)