Skip to content

Commit bf46140

Browse files
ondrejmirtesclaude
andcommitted
Settle the conditional targets before deriving the guards
createConditionalExpressions() derived a guard for every differing key, and since the guard type became the branch set difference that costs a TypeCombinator::remove() of the two branch types - the most expensive step of a merge. A guard is only ever consumed paired with a target: a different key in the first target loop, any key in the second. So the targets are settled first with the cheap checks, the merge returns early when no key can be one, and the subtraction is skipped for a guard whose only possible target is its own key, which the target loop unsets from its own guard set anyway. The first target loop reuses the settled set instead of repeating the checks. Analysing Slevomat: 603.1s -> 591.6s user CPU, with byte-identical output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDqrN1pcN3qWc7xzKfkJaF
1 parent b3547b6 commit bf46140

2 files changed

Lines changed: 108 additions & 33 deletions

File tree

‎src/Analyser/ScopeOps.php‎

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,42 @@ public static function createConditionalExpressions(
378378
{
379379
$newVariableTypes = $ourExpressionTypes;
380380

381+
// A guard is only ever consumed paired with a target: a *different* key in
382+
// the first target loop below, any key in the second one. Deriving a guard
383+
// costs a TypeCombinator::remove() of the two branch types - the most
384+
// expensive step of the merge - so settle which keys can be targets with
385+
// the cheap checks first, bail out when none can, and skip the subtraction
386+
// for a guard no target can pair with. The first target loop reuses this
387+
// set instead of repeating the checks.
388+
$targets = [];
389+
$hasUndefinedTarget = false;
390+
foreach (array_keys($differingKeys) as $exprString) {
391+
if (!array_key_exists($exprString, $newVariableTypes)) {
392+
if (
393+
array_key_exists($exprString, $mergedExpressionTypes)
394+
&& !$mergedExpressionTypes[$exprString]->getExpr() instanceof VirtualNode
395+
) {
396+
$hasUndefinedTarget = true;
397+
}
398+
continue;
399+
}
400+
$targetHolder = $newVariableTypes[$exprString];
401+
if ($targetHolder->getExpr() instanceof VirtualNode) {
402+
continue;
403+
}
404+
if (
405+
array_key_exists($exprString, $mergedExpressionTypes)
406+
&& $mergedExpressionTypes[$exprString]->equals($targetHolder)
407+
) {
408+
continue;
409+
}
410+
$targets[$exprString] = true;
411+
}
412+
if (!$hasUndefinedTarget && count($targets) === 0) {
413+
return $conditionalExpressions;
414+
}
415+
$onlySelfIsTarget = !$hasUndefinedTarget && count($targets) === 1;
416+
381417
// When our-branch type is a subtype of their-branch type, the union
382418
// absorbs it (merged === their). Such a variable cannot be a *guard* —
383419
// its branch set difference below comes out empty — but it remains a
@@ -441,6 +477,12 @@ public static function createConditionalExpressions(
441477
continue;
442478
}
443479

480+
if ($onlySelfIsTarget && array_key_exists($exprString, $targets)) {
481+
// the sole target is this very key, which the target loop unsets from
482+
// its own guard set - no pairing can use this guard
483+
continue;
484+
}
485+
444486
// The set difference between the branch types is the part of our type
445487
// the other branch cannot produce: observing it later proves this
446488
// branch was taken, even when the full branch types overlap - so a
@@ -476,20 +518,8 @@ public static function createConditionalExpressions(
476518
$guardIsSuperTypeOfTheirExprCache = [];
477519
$theirExprIsSuperTypeOfGuardCache = [];
478520

479-
foreach (array_keys($differingKeys) as $exprString) {
480-
if (!array_key_exists($exprString, $newVariableTypes)) {
481-
continue;
482-
}
521+
foreach ($targets as $exprString => $isTarget) {
483522
$holder = $newVariableTypes[$exprString];
484-
if ($holder->getExpr() instanceof VirtualNode) {
485-
continue;
486-
}
487-
if (
488-
array_key_exists($exprString, $mergedExpressionTypes)
489-
&& $mergedExpressionTypes[$exprString]->equals($holder)
490-
) {
491-
continue;
492-
}
493523

494524
$variableTypeGuards = $typeGuards;
495525
unset($variableTypeGuards[$exprString]);

‎turbo-ext/src/ScopeOps.cpp‎

Lines changed: 65 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,59 @@ class ScopeOps
476476
return zv::Val();
477477
}
478478

479+
/* A guard is only ever consumed paired with a target: a *different* key
480+
* in the first target loop below, any key in the second one. Deriving a
481+
* guard costs a TypeCombinator::remove() of the two branch types - the
482+
* most expensive step of the merge - so settle which keys can be targets
483+
* with the cheap checks first, bail out when none can, and skip the
484+
* subtraction for a guard no target can pair with. The first target loop
485+
* reuses this set instead of repeating the checks. Mirrors the twin. */
486+
zv::ScratchTable targets(8);
487+
bool hasUndefinedTarget = false;
488+
for (auto diffEntry : differingKeys) {
489+
zend_string *key = diffEntry.stringKeyOrNull();
490+
zend_ulong idx = diffEntry.indexKey();
491+
492+
zval *ourSlot = pt_ht_find(ours.table(), key, idx);
493+
zval *mergedSlot = pt_ht_find(merged.table(), key, idx);
494+
if (ourSlot == NULL) {
495+
if (mergedSlot != NULL) {
496+
zv::Ref mergedHolder = zv::Ref(mergedSlot).deref();
497+
if (UNEXPECTED(!pt_check_holder(mergedHolder.raw()))) {
498+
return zv::Val();
499+
}
500+
if (!instanceof_function(holderExpr(mergedHolder)->ce, virtualNodeCe)) {
501+
hasUndefinedTarget = true;
502+
}
503+
}
504+
continue;
505+
}
506+
zv::Ref holder = zv::Ref(ourSlot).deref();
507+
if (UNEXPECTED(!pt_check_holder(holder.raw()))) {
508+
return zv::Val();
509+
}
510+
if (instanceof_function(holderExpr(holder)->ce, virtualNodeCe)) {
511+
continue;
512+
}
513+
if (mergedSlot != NULL) {
514+
zv::Ref mergedHolder = zv::Ref(mergedSlot).deref();
515+
bool equal;
516+
if (UNEXPECTED(!pt_holder_equals(mergedHolder.raw(), holder.raw(), &equal))) {
517+
return zv::Val();
518+
}
519+
if (equal) {
520+
continue;
521+
}
522+
}
523+
zval targetTrue;
524+
ZVAL_TRUE(&targetTrue);
525+
pt_ht_update(targets.table(), key, idx, &targetTrue);
526+
}
527+
if (!hasUndefinedTarget && targets.size() == 0) {
528+
return zv::Arr::copyOfTable(conditional.table());
529+
}
530+
bool onlySelfIsTarget = !hasUndefinedTarget && targets.size() == 1;
531+
479532
zv::ScratchTable guardsToExclude(8);
480533
zv::ScratchTable typeGuards(8);
481534
/* owns the remainder-typed holders created below; the scratch table
@@ -587,6 +640,12 @@ class ScopeOps
587640
continue;
588641
}
589642

643+
if (onlySelfIsTarget && pt_ht_exists(targets.table(), key, idx)) {
644+
/* the sole target is this very key, which the target loop unsets
645+
* from its own guard set - no pairing can use this guard */
646+
continue;
647+
}
648+
590649
/* the branch set difference — see the twin for why an unchanged
591650
* remainder falls back to the merged-type comparison */
592651
zv::Val remainder = typeCombinatorRemove(holderType(holder), holderType(theirHolder));
@@ -641,32 +700,18 @@ class ScopeOps
641700
zv::ScratchTable guardIsConstantArrayCache(8);
642701
zv::Arr result; /* stays UNDEF until the first append duplicates the input */
643702

644-
/* main loop: pair non-merged expressions with guards */
645-
for (auto diffEntry : differingKeys) {
646-
zend_string *key = diffEntry.stringKeyOrNull();
647-
zend_ulong idx = diffEntry.indexKey();
703+
/* main loop: pair non-merged expressions with guards - the settled
704+
* target set already carries the cheap checks */
705+
for (auto targetEntry : zv::TableRef(targets.table())) {
706+
zend_string *key = targetEntry.stringKeyOrNull();
707+
zend_ulong idx = targetEntry.indexKey();
648708

649709
zval *ourSlot = pt_ht_find(ours.table(), key, idx);
650-
if (ourSlot == NULL) {
710+
if (UNEXPECTED(ourSlot == NULL)) {
651711
continue;
652712
}
653713
zv::Ref holder = zv::Ref(ourSlot).deref();
654714

655-
if (instanceof_function(holderExpr(holder)->ce, virtualNodeCe)) {
656-
continue;
657-
}
658-
zval *mergedSlot = pt_ht_find(merged.table(), key, idx);
659-
if (mergedSlot != NULL) {
660-
zv::Ref mergedHolder = zv::Ref(mergedSlot).deref();
661-
bool equal;
662-
if (UNEXPECTED(!pt_holder_equals(mergedHolder.raw(), holder.raw(), &equal))) {
663-
return zv::Val();
664-
}
665-
if (equal) {
666-
continue;
667-
}
668-
}
669-
670715
bool hasSelfGuard = pt_ht_exists(typeGuards.table(), key, idx);
671716
if (typeGuards.size() - (hasSelfGuard ? 1 : 0) == 0) {
672717
continue;

0 commit comments

Comments
 (0)