Make closure signature and static variable inference cheaper, fix benchmark regressions

.github/workflows/bench.yml

@@ -23,17 +23,10 @@ permissions:
 
 jobs:
   baseline:
-    name: "Baseline (PHP ${{ matrix.php-version }})"
+    name: "Baseline"
     runs-on: ubuntu-latest
     timeout-minutes: 60
 
-    strategy:
-      fail-fast: false
-      matrix:
-        php-version:
-          - "7.4"
-          - "8.5"
-
     steps:
       - name: Harden the runner (Audit all outbound calls)
         uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0
@@ -47,65 +40,31 @@ jobs:
         uses: "shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240" # 2.37.2
         with:
           coverage: "none"
-          php-version: "${{ matrix.php-version }}"
+          php-version: "8.5"
           tools: pecl
           extensions: ds,mbstring
           ini-file: development
           ini-values: memory_limit=-1
 
-      - name: "Downgrade PHPUnit"
-        if: matrix.php-version == '7.4'
-        run: "composer require --dev phpunit/phpunit:^9.6 sebastian/diff:^4.0 doctrine/instantiator:^1.0 --update-with-dependencies --ignore-platform-reqs"
-
       - uses: "ramsey/composer-install@65e4f84970763564f46a70b8a54b90d033b3bdda" # v4.0.0
-
-      # tests/composer.json pins the Composer platform to PHP 8.2, which would make
-      # Composer resolve PHPBench's Symfony dependencies to versions that cannot
-      # even be parsed on PHP 7.4. Unsetting it resolves against the real PHP version.
-      - name: "Downgrade PHPBench"
-        if: matrix.php-version == '7.4'
-        run: |
-          composer config --unset platform.php --working-dir=tests
-          composer require --dev phpbench/phpbench:1.2.14 phpunit/phpunit:^9.6 brianium/paratest:^6.5 --update-with-all-dependencies --working-dir=tests
-
       - uses: "ramsey/composer-install@65e4f84970763564f46a70b8a54b90d033b3bdda" # v4.0.0
-        if: matrix.php-version != '7.4'
         with:
           working-directory: "tests/"
 
-      - uses: ./.github/actions/downgrade-code
-        with:
-          php-version: "${{ matrix.php-version }}"
-
       - name: "Run phpbench baseline"
-        run: "tests/vendor/bin/phpbench run --dump-file=tests/bench/storage/baseline-${{ matrix.php-version }}.xml --ansi"
+        run: "tests/vendor/bin/phpbench run --dump-file=tests/bench/storage/baseline.xml --ansi"
 
-      # Before committing an artifact as the new baseline, compare the <baseline-machine>
-      # values (md5, file_rw) in it with the ones in the currently committed baseline.
-      # GitHub runners differ in speed by up to 2x, and a baseline recorded on a fast
-      # runner makes every later run look like a regression.
       - name: "Upload baseline artifact"
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
         with:
-          name: phpbench-baseline-${{ matrix.php-version }}
-          path: tests/bench/storage/baseline-${{ matrix.php-version }}.xml
+          name: phpbench-baseline
+          path: tests/bench/storage/baseline.xml
 
   test:
-    name: "Test (PHP ${{ matrix.php-version }})"
+    name: "Test"
     runs-on: ubuntu-latest
     timeout-minutes: 60
 
-    strategy:
-      fail-fast: false
-      matrix:
-        include:
-          # phpbench 1.2.14 is the last version supporting PHP 7.4
-          # and it does not have the aggregate-preview report 'my-report' extends
-          - php-version: "7.4"
-            report: "aggregate"
-          - php-version: "8.5"
-            report: "my-report"
-
     steps:
       - name: Harden the runner (Audit all outbound calls)
         uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0
@@ -119,42 +78,16 @@ jobs:
         uses: "shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240" # 2.37.2
         with:
           coverage: "none"
-          php-version: "${{ matrix.php-version }}"
+          php-version: "8.5"
           tools: pecl
           extensions: ds,mbstring
           ini-file: development
           ini-values: memory_limit=-1
 
-      - name: "Downgrade PHPUnit"
-        if: matrix.php-version == '7.4'
-        run: "composer require --dev phpunit/phpunit:^9.6 sebastian/diff:^4.0 doctrine/instantiator:^1.0 --update-with-dependencies --ignore-platform-reqs"
-
       - uses: "ramsey/composer-install@65e4f84970763564f46a70b8a54b90d033b3bdda" # v4.0.0
-
-      # tests/composer.json pins the Composer platform to PHP 8.2, which would make
-      # Composer resolve PHPBench's Symfony dependencies to versions that cannot
-      # even be parsed on PHP 7.4. Unsetting it resolves against the real PHP version.
-      - name: "Downgrade PHPBench"
-        if: matrix.php-version == '7.4'
-        run: |
-          composer config --unset platform.php --working-dir=tests
-          composer require --dev phpbench/phpbench:1.2.14 phpunit/phpunit:^9.6 brianium/paratest:^6.5 --update-with-all-dependencies --working-dir=tests
-
       - uses: "ramsey/composer-install@65e4f84970763564f46a70b8a54b90d033b3bdda" # v4.0.0
-        if: matrix.php-version != '7.4'
         with:
           working-directory: "tests/"
 
-      - uses: ./.github/actions/downgrade-code
-        with:
-          php-version: "${{ matrix.php-version }}"
-
-      # PHPBench does not fail when the baseline file is missing, it just runs
-      # with an empty baseline and every assertion passes
-      - name: "Check that the baseline exists"
-        run: |
-          test -f tests/bench/storage/baseline-${{ matrix.php-version }}.xml \
-            || (echo "Missing tests/bench/storage/baseline-${{ matrix.php-version }}.xml, download it from the phpbench-baseline-${{ matrix.php-version }} artifact of the Baseline job and commit it." && exit 1)
-
       - name: "Run phpbench test"
-        run: "tests/vendor/bin/phpbench run --file=tests/bench/storage/baseline-${{ matrix.php-version }}.xml --report=${{ matrix.report }} --ansi"
+        run: "tests/vendor/bin/phpbench run --file=tests/bench/storage/baseline.xml --report=my-report --ansi"

Makefile

@@ -15,6 +15,7 @@ lint:
 	XDEBUG_MODE=off php vendor/bin/parallel-lint --colors \
 		--exclude tests/PHPStan/Analyser/data \
 		--exclude tests/bench/data \
+		--exclude tests/bench/RegressionBench.php \
 		--exclude tests/PHPStan/Analyser/nsrt \
 		--exclude tests/PHPStan/Rules/Methods/data \
 		--exclude tests/PHPStan/Rules/Functions/data \

src/Analyser/ClosureProcessor.php

@@ -434,6 +434,60 @@ public function processByRefInvocation(
 		$enter = fn (MutatingScope $byRefSource): MutatingScope => $this->enterWithCapturedUses($scope, $expr, $callableParameters, $creationScope)->processClosureScope($byRefSource, null, $byRefUses);
 
 		$entryScope = $enter($scope);
+		$frame = $scope->getCurrentTemplateArgumentFrame();
+		$walk = null;
+		// the body entered in a state it was already walked from, in the same
+		// pass: a closure invoked many times mostly re-enters one of a few states
+		foreach ($storage->findByRefInvocationWalks($expr) as $storedWalk) {
+			[$walkFrame, $walkUntilFixpoint, $walkNativeTypesPromoted, $walkEntryScope] = $storedWalk;
+			if (
+				$walkFrame !== $frame
+				|| $walkUntilFixpoint !== $untilFixpoint
+				|| $walkNativeTypesPromoted !== $scope->nativeTypesPromoted
+				|| !$walkEntryScope->equals($entryScope)
+			) {
+				continue;
+			}
+			$walk = $storedWalk;
+			break;
+		}
+		if ($walk === null) {
+			[$exitScope, $bodyThrowPoints] = $this->walkByRefInvocation($nodeScopeResolver, $expr, $scope, $storage, $entryScope, $enter, $byRefUses, $untilFixpoint);
+			$walk = [$frame, $untilFixpoint, $scope->nativeTypesPromoted, $entryScope, $exitScope, $bodyThrowPoints];
+			$storage->storeByRefInvocationWalk($expr, $walk);
+		}
+		[, , , , $exitScope, $bodyThrowPoints] = $walk;
+
+		$throwPoints = [];
+		foreach ($bodyThrowPoints as $throwPoint) {
+			$throwScope = $this->assignByRefUses($scope, $throwPoint->getScope(), $byRefUses);
+			$throwPoints[] = $throwPoint->isExplicit()
+				? InternalThrowPoint::createExplicit($throwScope, $throwPoint->getType(), $call, $throwPoint->canContainAnyThrowable())
+				: InternalThrowPoint::createImplicit($throwScope, $call);
+		}
+
+		return [$this->assignByRefUses($scope, $exitScope, $byRefUses), $throwPoints];
+	}
+
+	/**
+	 * The walks of processByRefInvocation() from the entry scope: the body's
+	 * exit scope and throw points, in the closure's own scope.
+	 *
+	 * @param callable(MutatingScope): MutatingScope $enter
+	 * @param Node\ClosureUse[] $byRefUses
+	 * @return array{MutatingScope, list<InternalThrowPoint>}
+	 */
+	private function walkByRefInvocation(
+		NodeScopeResolver $nodeScopeResolver,
+		Expr\Closure $expr,
+		MutatingScope $scope,
+		ExpressionResultStorage $storage,
+		MutatingScope $entryScope,
+		callable $enter,
+		array $byRefUses,
+		bool $untilFixpoint,
+	): array
+	{
 		$exitScope = null;
 		$throwPoints = [];
 		$count = 0;
@@ -445,10 +499,7 @@ public function processByRefInvocation(
 			}
 			$exitScope = $exitScope === null ? $passExitScope : $exitScope->mergeWith($passExitScope);
 			foreach ($result->getThrowPoints() as $throwPoint) {
-				$throwScope = $this->assignByRefUses($scope, $throwPoint->getScope(), $byRefUses);
-				$throwPoints[] = $throwPoint->isExplicit()
-					? InternalThrowPoint::createExplicit($throwScope, $throwPoint->getType(), $call, $throwPoint->canContainAnyThrowable())
-					: InternalThrowPoint::createImplicit($throwScope, $call);
+				$throwPoints[] = $throwPoint;
 			}
 			if (!$untilFixpoint) {
 				break;
@@ -471,7 +522,7 @@ public function processByRefInvocation(
 			$exitScope = $exitScope->mergeWith($entryScope);
 		}
 
-		return [$this->assignByRefUses($scope, $exitScope, $byRefUses), $throwPoints];
+		return [$exitScope, $throwPoints];
 	}
 
 	/**

src/Analyser/ExprHandler/ArrayHandler.php

@@ -225,11 +225,8 @@ private function collectAbsorbedItems(Array_ $expr, array $itemResults, Mutating
 		}
 
 		$arrayType = $this->initializerExprTypeResolver->getArrayType($expr, static fn (Expr $inner): Type => $itemResults[spl_object_id($inner)]->getType());
-		foreach ($itemTypes as $itemType) {
-			$scope = $scope->addTemplateArgumentConstraints(ClosureSignatureInference::collectAbsorbed($itemType, $arrayType));
-		}
 
-		return $scope;
+		return $scope->addTemplateArgumentConstraints(ClosureSignatureInference::collectAbsorbedInto($itemTypes, $arrayType));
 	}
 
 	private function getExpectedArrayType(?Type $type): ?Type

src/Analyser/ExprHandler/IssetHandler.php

@@ -227,7 +227,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 						$rightFalseyScope = $accTruthyScope->applySpecifiedTypes($rightTypes($accTruthyScope, TypeSpecifierContext::createFalsey()));
 
 						$leftExprNode = $accExpr;
-						$leftTypes = $accTypes;
+						$leftTypes = self::memoizeSubjectTypes($accTypes);
 						$leftTruthyScope = $accTruthyScope;
 						$leftFalseyScope = $accFalseyScope;
 						$accTypes = fn (MutatingScope $scope, TypeSpecifierContext $ctx): SpecifiedTypes => $this->booleanNarrowingHelper->specifyConjunction(
@@ -264,4 +264,31 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 		);
 	}
 
+	/**
+	 * The narrowing of the conjunction folded so far: every later conjunction
+	 * asks it again as its left operand, with the scopes and contexts it was
+	 * already asked with - answered once per pair instead of re-folding every
+	 * subject before it on each ask.
+	 *
+	 * @param Closure(MutatingScope, TypeSpecifierContext): SpecifiedTypes $types
+	 * @return Closure(MutatingScope, TypeSpecifierContext): SpecifiedTypes
+	 */
+	private static function memoizeSubjectTypes(Closure $types): Closure
+	{
+		$answered = [];
+
+		return static function (MutatingScope $scope, TypeSpecifierContext $context) use ($types, &$answered): SpecifiedTypes {
+			foreach ($answered as [$answeredScope, $answeredContext, $answer]) {
+				if ($answeredScope === $scope && $answeredContext === $context) {
+					return $answer;
+				}
+			}
+
+			$answer = $types($scope, $context);
+			$answered[] = [$scope, $context, $answer];
+
+			return $answer;
+		};
+	}
+
 }

src/Analyser/ExpressionResultStorage.php

@@ -3,6 +3,7 @@
 namespace PHPStan\Analyser;
 
 use PhpParser\Node\Expr;
+use PHPStan\Analyser\Generics\TemplateArgumentFrame;
 use PHPStan\Turbo\ShadowedByTurboExtension;
 use SplObjectStorage;
 
@@ -19,6 +20,17 @@ final class ExpressionResultStorage
 	 */
 	private ?self $fallback = null;
 
+	/**
+	 * The walks of closure bodies run for invocations of closures whose by-ref
+	 * uses are followed (see ClosureProcessor::processByRefInvocation()),
+	 * keyed by the closure: [frame, until fixpoint, native types promoted,
+	 * entry scope, exit scope, throw points]. The walk is a function of these,
+	 * so an invocation entering the body in a state already walked reads it.
+	 *
+	 * @var SplObjectStorage<Expr\Closure, list<array{TemplateArgumentFrame|null, bool, bool, MutatingScope, MutatingScope, list<InternalThrowPoint>}>>|null
+	 */
+	private ?SplObjectStorage $byRefInvocationWalks = null;
+
 	public function __construct()
 	{
 		$this->exprResults = new SplObjectStorage();
@@ -46,4 +58,30 @@ public function findExpressionResult(Expr $expr): ?ExpressionResult
 		return $this->exprResults[$expr] ?? ($this->fallback !== null ? $this->fallback->findExpressionResult($expr) : null);
 	}
 
+	/**
+	 * @param array{TemplateArgumentFrame|null, bool, bool, MutatingScope, MutatingScope, list<InternalThrowPoint>} $walk
+	 */
+	public function storeByRefInvocationWalk(Expr\Closure $closure, array $walk): void
+	{
+		$this->byRefInvocationWalks ??= new SplObjectStorage();
+		$walks = $this->byRefInvocationWalks[$closure] ?? [];
+		$walks[] = $walk;
+		$this->byRefInvocationWalks[$closure] = $walks;
+	}
+
+	/**
+	 * The walks stored here and in the storages this one falls back to.
+	 *
+	 * @return list<array{TemplateArgumentFrame|null, bool, bool, MutatingScope, MutatingScope, list<InternalThrowPoint>}>
+	 */
+	public function findByRefInvocationWalks(Expr\Closure $closure): array
+	{
+		$walks = $this->byRefInvocationWalks[$closure] ?? [];
+		if ($this->fallback === null) {
+			return $walks;
+		}
+
+		return [...$walks, ...$this->fallback->findByRefInvocationWalks($closure)];
+	}
+
 }

src/Analyser/Generics/ClosureSignatureInference.php

@@ -289,21 +289,38 @@ public static function collectInvokedCallee(Type $calleeType): TemplateArgumentC
 	 * they escape.
 	 */
 	public static function collectAbsorbed(Type $input, Type $result): TemplateArgumentConstraints
+	{
+		return self::collectAbsorbedInto([$input], $result);
+	}
+
+	/**
+	 * The values of $inputs merged into $result - the items of an array
+	 * literal, the members of a union. See collectAbsorbed(). The markers
+	 * $result kept are collected once for all of them: a large literal of
+	 * closure-carrying items would otherwise traverse the whole array type
+	 * once per item.
+	 *
+	 * @param list<Type> $inputs
+	 */
+	public static function collectAbsorbedInto(array $inputs, Type $result): TemplateArgumentConstraints
 	{
 		$constraints = TemplateArgumentConstraints::createEmpty();
-		if ($input === $result) {
-			return $constraints;
-		}
-		$markers = self::collectMarkers($input);
-		if ($markers === []) {
-			return $constraints;
-		}
-		$kept = self::collectMarkers($result);
-		foreach ($markers as $key => $marker) {
-			if (isset($kept[$key])) {
+		$kept = null;
+		foreach ($inputs as $input) {
+			if ($input === $result) {
 				continue;
 			}
-			$constraints = $constraints->withUnconstrainingSend($marker);
+			$markers = self::collectMarkers($input);
+			if ($markers === []) {
+				continue;
+			}
+			$kept ??= self::collectMarkers($result);
+			foreach ($markers as $key => $marker) {
+				if (isset($kept[$key])) {
+					continue;
+				}
+				$constraints = $constraints->withUnconstrainingSend($marker);
+			}
 		}
 
 		return $constraints;
@@ -330,12 +347,7 @@ public static function collectAbsorbedInUnion(array $types): TemplateArgumentCon
 			return $constraints;
 		}
 
-		$union = TypeCombinator::union(...$types);
-		foreach ($types as $type) {
-			$constraints = $constraints->merge(self::collectAbsorbed($type, $union));
-		}
-
-		return $constraints;
+		return self::collectAbsorbedInto($types, TypeCombinator::union(...$types));
 	}
 
 	/** Whether $type carries a closure written where nothing types it. */