Skip to content

Commit 86312b7

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Create merge conditionals from the differing holders only
createConditionalExpressions() scanned every holder map in full - twice per merge - although each of its loops only ever selects entries whose our/their/merged holders differ. mergeVariableHolders() knows exactly which keys those are and now reports them; the conditional creation iterates just that diff - typically a handful of keys instead of hundreds of holders. The ScopeOps turbo twin mirrors both signature changes. createConditionalExpressions was the largest self-time item in the single-pass branch's SPX profile; make phpstan user CPU measured -2..5% here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
1 parent 366cc05 commit 86312b7

5 files changed

Lines changed: 131 additions & 37 deletions

File tree

‎src/Analyser/MutatingScope.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3530,7 +3530,8 @@ public function mergeWith(?self $otherScope, bool $preserveVacuousConditionals =
35303530
$ourExpressionTypes = $this->expressionTypes;
35313531
$theirExpressionTypes = $otherScope->expressionTypes;
35323532

3533-
$mergedExpressionTypes = ScopeOps::mergeVariableHolders($ourExpressionTypes, $theirExpressionTypes);
3533+
$differingExpressionKeys = [];
3534+
$mergedExpressionTypes = ScopeOps::mergeVariableHolders($ourExpressionTypes, $theirExpressionTypes, $differingExpressionKeys);
35343535
$conditionalExpressions = ScopeOps::intersectConditionalExpressions($this->conditionalExpressions, $otherScope->conditionalExpressions);
35353536
if ($preserveVacuousConditionals) {
35363537
$conditionalExpressions = $this->preserveVacuousConditionalExpressions(
@@ -3549,12 +3550,14 @@ public function mergeWith(?self $otherScope, bool $preserveVacuousConditionals =
35493550
$ourExpressionTypes,
35503551
$theirExpressionTypes,
35513552
$mergedExpressionTypes,
3553+
$differingExpressionKeys,
35523554
);
35533555
$conditionalExpressions = ScopeOps::createConditionalExpressions(
35543556
$conditionalExpressions,
35553557
$theirExpressionTypes,
35563558
$ourExpressionTypes,
35573559
$mergedExpressionTypes,
3560+
$differingExpressionKeys,
35583561
);
35593562

35603563
[$mergedExpressionTypes, $mergedNativeTypes] = ScopeOps::finishMerge(

‎src/Analyser/ScopeOps.php‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use function array_filter;
2727
use function array_key_exists;
2828
use function array_key_first;
29+
use function array_keys;
2930
use function array_slice;
3031
use function count;
3132
use function get_class;
@@ -195,9 +196,10 @@ public static function scopeWith(
195196
*
196197
* @param array<string, ExpressionTypeHolder> $ourVariableTypeHolders
197198
* @param array<string, ExpressionTypeHolder> $theirVariableTypeHolders
199+
* @param array<string, true> $differingKeys
198200
* @return array<string, ExpressionTypeHolder>
199201
*/
200-
public static function mergeVariableHolders(array $ourVariableTypeHolders, array $theirVariableTypeHolders): array
202+
public static function mergeVariableHolders(array $ourVariableTypeHolders, array $theirVariableTypeHolders, array &$differingKeys = []): array
201203
{
202204
$intersectedVariableTypeHolders = [];
203205
$globalVariableCallback = static fn (Node $node) => $node instanceof Variable && is_string($node->name) && in_array($node->name, Scope::SUPERGLOBAL_VARIABLES, true);
@@ -209,8 +211,10 @@ public static function mergeVariableHolders(array $ourVariableTypeHolders, array
209211
continue;
210212
}
211213

214+
$differingKeys[$exprString] = true;
212215
$intersectedVariableTypeHolders[$exprString] = $variableTypeHolder->and($theirVariableTypeHolders[$exprString]);
213216
} else {
217+
$differingKeys[$exprString] = true;
214218
$expr = $variableTypeHolder->getExpr();
215219

216220
$containsSuperGlobal = $expr->getAttribute(self::CONTAINS_SUPER_GLOBAL_ATTRIBUTE_NAME);
@@ -231,6 +235,7 @@ public static function mergeVariableHolders(array $ourVariableTypeHolders, array
231235
continue;
232236
}
233237

238+
$differingKeys[$exprString] = true;
234239
$expr = $variableTypeHolder->getExpr();
235240

236241
$containsSuperGlobal = $expr->getAttribute(self::CONTAINS_SUPER_GLOBAL_ATTRIBUTE_NAME);
@@ -358,13 +363,15 @@ public static function intersectConditionalExpressions(array $ourConditionalExpr
358363
* @param array<string, ExpressionTypeHolder> $ourExpressionTypes
359364
* @param array<string, ExpressionTypeHolder> $theirExpressionTypes
360365
* @param array<string, ExpressionTypeHolder> $mergedExpressionTypes
366+
* @param array<string, true> $differingKeys
361367
* @return array<string, ConditionalExpressionHolder[]>
362368
*/
363369
public static function createConditionalExpressions(
364370
array $conditionalExpressions,
365371
array $ourExpressionTypes,
366372
array $theirExpressionTypes,
367373
array $mergedExpressionTypes,
374+
array $differingKeys,
368375
): array
369376
{
370377
$newVariableTypes = $ourExpressionTypes;
@@ -375,7 +382,11 @@ public static function createConditionalExpressions(
375382
// branch — but it remains a valid conditional *target*, so only exclude
376383
// it from guard selection instead of dropping it entirely.
377384
$guardsToExclude = [];
378-
foreach ($theirExpressionTypes as $exprString => $holder) {
385+
foreach (array_keys($differingKeys) as $exprString) {
386+
if (!array_key_exists($exprString, $theirExpressionTypes)) {
387+
continue;
388+
}
389+
$holder = $theirExpressionTypes[$exprString];
379390
if (!array_key_exists($exprString, $mergedExpressionTypes)) {
380391
continue;
381392
}
@@ -396,7 +407,11 @@ public static function createConditionalExpressions(
396407
}
397408

398409
$typeGuards = [];
399-
foreach ($newVariableTypes as $exprString => $holder) {
410+
foreach (array_keys($differingKeys) as $exprString) {
411+
if (!array_key_exists($exprString, $newVariableTypes)) {
412+
continue;
413+
}
414+
$holder = $newVariableTypes[$exprString];
400415
if ($holder->getExpr() instanceof VirtualNode) {
401416
continue;
402417
}
@@ -436,7 +451,11 @@ public static function createConditionalExpressions(
436451
$guardIsSuperTypeOfTheirExprCache = [];
437452
$theirExprIsSuperTypeOfGuardCache = [];
438453

439-
foreach ($newVariableTypes as $exprString => $holder) {
454+
foreach (array_keys($differingKeys) as $exprString) {
455+
if (!array_key_exists($exprString, $newVariableTypes)) {
456+
continue;
457+
}
458+
$holder = $newVariableTypes[$exprString];
440459
if ($holder->getExpr() instanceof VirtualNode) {
441460
continue;
442461
}
@@ -497,7 +516,11 @@ public static function createConditionalExpressions(
497516
}
498517
}
499518

500-
foreach ($mergedExpressionTypes as $exprString => $mergedExprTypeHolder) {
519+
foreach (array_keys($differingKeys) as $exprString) {
520+
if (!array_key_exists($exprString, $mergedExpressionTypes)) {
521+
continue;
522+
}
523+
$mergedExprTypeHolder = $mergedExpressionTypes[$exprString];
501524
if (array_key_exists($exprString, $ourExpressionTypes)) {
502525
continue;
503526
}

‎src/Turbo/TurboExtensionEnabler.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ final class TurboExtensionEnabler
2020
* version is the short SHA of the last commit touching turbo-ext/src/,
2121
* enforced by the phar.yml turbo-version job.
2222
*/
23-
public const EXPECTED_EXTENSION_VERSION = 'e9ce4dc';
23+
public const EXPECTED_EXTENSION_VERSION = 'b7ed7b5';
2424

2525
private static bool $typeCombinatorCacheEnabled = false;
2626

‎turbo-ext/src/ScopeOps.cpp‎

Lines changed: 82 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -311,11 +311,15 @@ class ScopeOps
311311
return zv::Val::adopt(out);
312312
}
313313

314-
/* Mirrors ScopeOps::mergeVariableHolders(). */
315-
static zv::Val mergeVariableHolders(zv::TableRef ours, zv::TableRef theirs)
314+
/*
315+
* Mirrors ScopeOps::mergeVariableHolders(). differing (nullable) receives
316+
* a true marker for every key that is not one shared holder on both sides
317+
* — the twin's &$differingKeys out-parameter.
318+
*/
319+
static zv::Val mergeVariableHolders(zv::TableRef ours, zv::TableRef theirs, HashTable *differing)
316320
{
317321
zv::Arr merged = zv::Arr::create(ours.size());
318-
if (UNEXPECTED(!mergeVariableHoldersInto(merged, ours, theirs))) {
322+
if (UNEXPECTED(!mergeVariableHoldersInto(merged, ours, theirs, differing))) {
319323
return zv::Val();
320324
}
321325
return zv::Val(std::move(merged));
@@ -394,7 +398,7 @@ class ScopeOps
394398

395399
/* mergedNative += filter(mergeVariableHolders(oursRemaining, theirsRemaining)) */
396400
{
397-
zv::Val remainingMerged = mergeVariableHolders(zv::TableRef(oursNativeRemaining.table()), zv::TableRef(theirsNativeRemaining.table()));
401+
zv::Val remainingMerged = mergeVariableHolders(zv::TableRef(oursNativeRemaining.table()), zv::TableRef(theirsNativeRemaining.table()), NULL);
398402
if (UNEXPECTED(remainingMerged.isUndef())) {
399403
return zv::Val();
400404
}
@@ -461,7 +465,7 @@ class ScopeOps
461465
* per-guard caches; the input array is only duplicated once the first
462466
* conditional is actually appended.
463467
*/
464-
static zv::Val createConditionalExpressions(zv::TableRef conditional, zv::TableRef ours, zv::TableRef theirs, zv::TableRef merged)
468+
static zv::Val createConditionalExpressions(zv::TableRef conditional, zv::TableRef ours, zv::TableRef theirs, zv::TableRef merged, zv::TableRef differingKeys)
465469
{
466470
zend_class_entry *virtualNodeCe = pt_class(PT_CLASS_VIRTUAL_NODE);
467471
if (UNEXPECTED(virtualNodeCe == NULL)) {
@@ -472,16 +476,22 @@ class ScopeOps
472476
zv::ScratchTable typeGuards(8);
473477

474478
/* guardsToExclude: subtype-absorbed their-branch variables are poor
475-
* guards but stay valid conditional targets */
476-
for (auto entry : theirs) {
477-
zend_string *key = entry.stringKeyOrNull();
478-
zend_ulong idx = entry.indexKey();
479+
* guards but stay valid conditional targets. Only the merge's differing
480+
* keys can qualify — iterate those (in their insertion order, like the
481+
* twin) instead of the whole holder maps. */
482+
for (auto diffEntry : differingKeys) {
483+
zend_string *key = diffEntry.stringKeyOrNull();
484+
zend_ulong idx = diffEntry.indexKey();
479485

486+
zval *theirSlot = pt_ht_find(theirs.table(), key, idx);
487+
if (theirSlot == NULL) {
488+
continue;
489+
}
480490
zval *mergedSlot = pt_ht_find(merged.table(), key, idx);
481491
if (mergedSlot == NULL) {
482492
continue;
483493
}
484-
zv::Ref holder = entry.value().deref();
494+
zv::Ref holder = zv::Ref(theirSlot).deref();
485495
if (UNEXPECTED(!pt_check_holder(holder.raw()))) {
486496
return zv::Val();
487497
}
@@ -522,11 +532,15 @@ class ScopeOps
522532
}
523533

524534
/* typeGuards */
525-
for (auto entry : ours) {
526-
zend_string *key = entry.stringKeyOrNull();
527-
zend_ulong idx = entry.indexKey();
528-
zv::Ref holder = entry.value().deref();
535+
for (auto diffEntry : differingKeys) {
536+
zend_string *key = diffEntry.stringKeyOrNull();
537+
zend_ulong idx = diffEntry.indexKey();
529538

539+
zval *ourSlot = pt_ht_find(ours.table(), key, idx);
540+
if (ourSlot == NULL) {
541+
continue;
542+
}
543+
zv::Ref holder = zv::Ref(ourSlot).deref();
530544
if (UNEXPECTED(!pt_check_holder(holder.raw()))) {
531545
return zv::Val();
532546
}
@@ -585,10 +599,15 @@ class ScopeOps
585599
zv::Arr result; /* stays UNDEF until the first append duplicates the input */
586600

587601
/* main loop: pair non-merged expressions with guards */
588-
for (auto entry : ours) {
589-
zend_string *key = entry.stringKeyOrNull();
590-
zend_ulong idx = entry.indexKey();
591-
zv::Ref holder = entry.value().deref();
602+
for (auto diffEntry : differingKeys) {
603+
zend_string *key = diffEntry.stringKeyOrNull();
604+
zend_ulong idx = diffEntry.indexKey();
605+
606+
zval *ourSlot = pt_ht_find(ours.table(), key, idx);
607+
if (ourSlot == NULL) {
608+
continue;
609+
}
610+
zv::Ref holder = zv::Ref(ourSlot).deref();
592611

593612
if (instanceof_function(holderExpr(holder)->ce, virtualNodeCe)) {
594613
continue;
@@ -711,14 +730,18 @@ class ScopeOps
711730
}
712731

713732
/* their-only expressions: record certainty-No conditionals per guard */
714-
for (auto entry : merged) {
715-
zend_string *key = entry.stringKeyOrNull();
716-
zend_ulong idx = entry.indexKey();
733+
for (auto diffEntry : differingKeys) {
734+
zend_string *key = diffEntry.stringKeyOrNull();
735+
zend_ulong idx = diffEntry.indexKey();
717736

737+
zval *mergedSlot = pt_ht_find(merged.table(), key, idx);
738+
if (mergedSlot == NULL) {
739+
continue;
740+
}
718741
if (pt_ht_exists(ours.table(), key, idx)) {
719742
continue;
720743
}
721-
zv::Ref mergedHolder = entry.value().deref();
744+
zv::Ref mergedHolder = zv::Ref(mergedSlot).deref();
722745
if (UNEXPECTED(!pt_check_holder(mergedHolder.raw()))) {
723746
return zv::Val();
724747
}
@@ -1381,8 +1404,19 @@ class ScopeOps
13811404
return zv::Val::adopt(created);
13821405
}
13831406

1407+
/* $differing[$key] = true (marker insert, overwrites) */
1408+
static void markDiffering(HashTable *differing, zend_string *skey, zend_ulong idx)
1409+
{
1410+
if (differing == NULL) {
1411+
return;
1412+
}
1413+
zval trueZv;
1414+
ZVAL_TRUE(&trueZv);
1415+
pt_ht_update(differing, skey, idx, &trueZv);
1416+
}
1417+
13841418
/* The two loops of mergeVariableHolders(), filling a caller-owned table. */
1385-
static bool mergeVariableHoldersInto(zv::Arr &merged, zv::TableRef ours, zv::TableRef theirs)
1419+
static bool mergeVariableHoldersInto(zv::Arr &merged, zv::TableRef ours, zv::TableRef theirs, HashTable *differing)
13861420
{
13871421
for (auto entry : ours) {
13881422
zend_string *key = entry.stringKeyOrNull();
@@ -1402,13 +1436,15 @@ class ScopeOps
14021436
if (holder.asObject() == theirHolder.asObject()) {
14031437
tableAddNewCopy(merged.table(), key, idx, holder);
14041438
} else {
1439+
markDiffering(differing, key, idx);
14051440
zval andHolder;
14061441
if (UNEXPECTED(!pt_holder_and(holder.raw(), theirHolder.raw(), &andHolder))) {
14071442
return false;
14081443
}
14091444
tableAddNew(merged.table(), key, idx, zv::Val::adopt(andHolder));
14101445
}
14111446
} else {
1447+
markDiffering(differing, key, idx);
14121448
bool containsSuperGlobal = pt_expr_contains_superglobal(holderExpr(holder));
14131449
if (UNEXPECTED(EG(exception))) {
14141450
return false;
@@ -1427,6 +1463,7 @@ class ScopeOps
14271463
if (pt_ht_exists(merged.table(), key, idx)) {
14281464
continue;
14291465
}
1466+
markDiffering(differing, key, idx);
14301467
zv::Ref holder = entry.value().deref();
14311468
if (UNEXPECTED(!pt_check_holder(holder.raw()))) {
14321469
return false;
@@ -2060,13 +2097,27 @@ void pt_register_scope_ops()
20602097
{
20612098
reg::Class cls("PHPStanTurbo\\ScopeOps");
20622099

2063-
cls.method("mergeVariableHolders", reg::PublicStatic, 2, { reg::arrayArg("ourVariableTypeHolders"), reg::arrayArg("theirVariableTypeHolders") }, [](INTERNAL_FUNCTION_PARAMETERS) {
2100+
cls.method("mergeVariableHolders", reg::PublicStatic, 2, { reg::arrayArg("ourVariableTypeHolders"), reg::arrayArg("theirVariableTypeHolders"), reg::any("differingKeys", true) }, [](INTERNAL_FUNCTION_PARAMETERS) {
20642101
HashTable *ours, *theirs;
2065-
ZEND_PARSE_PARAMETERS_START(2, 2)
2102+
zval *differing_zv = NULL;
2103+
ZEND_PARSE_PARAMETERS_START(2, 3)
20662104
Z_PARAM_ARRAY_HT(ours)
20672105
Z_PARAM_ARRAY_HT(theirs)
2106+
Z_PARAM_OPTIONAL
2107+
Z_PARAM_ZVAL(differing_zv)
20682108
ZEND_PARSE_PARAMETERS_END();
2069-
zv::Val result = ScopeOps::mergeVariableHolders(zv::TableRef(ours), zv::TableRef(theirs));
2109+
HashTable *differing = NULL;
2110+
if (differing_zv != NULL && Z_ISREF_P(differing_zv)) {
2111+
/* the twin declares `array &$differingKeys = []`; vivify like PHP
2112+
* would and write through the reference */
2113+
zval *inner = Z_REFVAL_P(differing_zv);
2114+
if (Z_TYPE_P(inner) != IS_ARRAY) {
2115+
convert_to_array(inner);
2116+
}
2117+
SEPARATE_ARRAY(inner);
2118+
differing = Z_ARRVAL_P(inner);
2119+
}
2120+
zv::Val result = ScopeOps::mergeVariableHolders(zv::TableRef(ours), zv::TableRef(theirs), differing);
20702121
if (UNEXPECTED(result.isUndef())) {
20712122
RETURN_THROWS();
20722123
}
@@ -2184,15 +2235,16 @@ void pt_register_scope_ops()
21842235
result.intoReturnValue(return_value);
21852236
});
21862237

2187-
cls.method("createConditionalExpressions", reg::PublicStatic, 4, { reg::arrayArg("conditionalExpressions"), reg::arrayArg("ourExpressionTypes"), reg::arrayArg("theirExpressionTypes"), reg::arrayArg("mergedExpressionTypes") }, [](INTERNAL_FUNCTION_PARAMETERS) {
2188-
HashTable *conditional, *ours, *theirs, *merged;
2189-
ZEND_PARSE_PARAMETERS_START(4, 4)
2238+
cls.method("createConditionalExpressions", reg::PublicStatic, 5, { reg::arrayArg("conditionalExpressions"), reg::arrayArg("ourExpressionTypes"), reg::arrayArg("theirExpressionTypes"), reg::arrayArg("mergedExpressionTypes"), reg::arrayArg("differingKeys") }, [](INTERNAL_FUNCTION_PARAMETERS) {
2239+
HashTable *conditional, *ours, *theirs, *merged, *differing_keys;
2240+
ZEND_PARSE_PARAMETERS_START(5, 5)
21902241
Z_PARAM_ARRAY_HT(conditional)
21912242
Z_PARAM_ARRAY_HT(ours)
21922243
Z_PARAM_ARRAY_HT(theirs)
21932244
Z_PARAM_ARRAY_HT(merged)
2245+
Z_PARAM_ARRAY_HT(differing_keys)
21942246
ZEND_PARSE_PARAMETERS_END();
2195-
zv::Val result = ScopeOps::createConditionalExpressions(zv::TableRef(conditional), zv::TableRef(ours), zv::TableRef(theirs), zv::TableRef(merged));
2247+
zv::Val result = ScopeOps::createConditionalExpressions(zv::TableRef(conditional), zv::TableRef(ours), zv::TableRef(theirs), zv::TableRef(merged), zv::TableRef(differing_keys));
21962248
if (UNEXPECTED(result.isUndef())) {
21972249
RETURN_THROWS();
21982250
}

‎turbo-ext/tests/smoke.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,22 @@ function check(bool $cond, string $msg): void
337337
check($storage->pendingFibers === [], "ERS $label: fiber array entries can be unset");
338338
}
339339

340+
// ---- ScopeOps::mergeVariableHolders differingKeys ----
341+
$sharedP = $pH($expr1, $int, $pYes);
342+
$sharedN = $nH($expr1, $int, $nYes);
343+
$mergePOurs = ['$shared' => $sharedP, '$a' => $pH($expr1, $int, $pYes), '$b' => $pH($expr2, $string, $pYes)];
344+
$mergePTheirs = ['$shared' => $sharedP, '$b' => $pH($expr2, $string, $pMaybe), '$c' => $pH($expr2, $int, $pYes)];
345+
$mergeNOurs = ['$shared' => $sharedN, '$a' => $nH($expr1, $int, $nYes), '$b' => $nH($expr2, $string, $nYes)];
346+
$mergeNTheirs = ['$shared' => $sharedN, '$b' => $nH($expr2, $string, $nMaybe), '$c' => $nH($expr2, $int, $nYes)];
347+
$pDiffering = [];
348+
$pMerged = \PHPStan\Analyser\ScopeOps::mergeVariableHolders($mergePOurs, $mergePTheirs, $pDiffering);
349+
$nDiffering = [];
350+
$nMerged = \PHPStanTurbo\ScopeOps::mergeVariableHolders($mergeNOurs, $mergeNTheirs, $nDiffering);
351+
check($pDiffering === $nDiffering, 'ScopeOps mergeVariableHolders differingKeys parity: ' . json_encode($pDiffering) . ' vs ' . json_encode($nDiffering));
352+
check(array_keys($pMerged) === array_keys($nMerged), 'ScopeOps mergeVariableHolders merged keys parity');
353+
check(array_keys(\PHPStanTurbo\ScopeOps::mergeVariableHolders($mergeNOurs, $mergeNTheirs)) === array_keys($nMerged), 'ScopeOps mergeVariableHolders without differingKeys');
354+
355+
340356
// ---- NodeScanner ----
341357
$covered[\PHPStan\Node\NodeScanner::class] = true;
342358
$smokeParserFactory = new \PhpParser\ParserFactory();

0 commit comments

Comments
 (0)