FNSR ExpressionResultStorage

[ondrejmirtes]

[staabm]

issue bot results look outstanding - too good to be real 🥸

[ondrejmirtes]

Lots of diffs doesn't mean lots of bugs fixed 😀 The analysis is still too wrong.

[ondrejmirtes]

Alright, so this issue-bot report is far more realistic and closer to the end results. Solving issues like https://phpstan.org/r/8d3536cc-d647-455c-8919-300cc35d7abf and https://phpstan.org/r/59b9367b-8f29-4008-8536-91dca00362df is what I had in mind.

[ondrejmirtes]

I’m sure it’s used. It overrides an empty method from NodeScopeResolver.

Ondřej Mirtes

On Fri 19. 12. 2025 at 7:30, Markus Staab @.***> wrote:

@.**** commented on this pull request.

In src/Analyser/Fiber/FiberNodeScopeResolver.php https://github.com/phpstan/phpstan-src/pull/4628#discussion_r2633846328:

•   		$request->scope->toMutatingScope(),
  

• 		$storage,
  

• 		new NoopNodeCallback(),
  

• 		ExpressionContext::createTopLevel(),
  

• 	);
  

• 	if ($storage->findResult($request->expr) === null) {
  

• 		throw new ShouldNotHappenException(sprintf('processExprNode should have stored the result of %s on line %s', get_class($request->expr), $request->expr->getStartLine()));
  

• 	}
  

• 	$this->processPendingFibers($storage);
  

• 	// Break and restart the loop since the array may have been modified
  

• 	return;
  

• }
  

• }
• protected function processPendingFibersForRequestedExpr(ExpressionResultStorage $storage, Expr $expr, ExpressionResult $result): void

this method seems unused

— Reply to this email directly, view it on GitHub https://github.com/phpstan/phpstan-src/pull/4628#pullrequestreview-3597086177, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZTOFVYCLK67LVP4DOC2L4COLQPAVCNFSM6AAAAACOW6DOEWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTKOJXGA4DMMJXG4 . You are receiving this because you authored the thread.Message ID: @.***>

[ondrejmirtes]

@staabm Alright, I have some performance profiles you could take a look at.

Here's 2.1.x: https://blackfire.io/profiles/86fcac97-986a-4e41-92e0-b41aaf53fbcc/graph Here's fnsr-storage without PHPSTAN_FNSR env variable: https://blackfire.io/profiles/cfa19ede-14e9-4f37-8961-d45d6f9b570d/graph (the difference should only be the fact we have ExpressionResultStorage with the results) Here's fnsr-storage with PHPSTAN_FNSR=1: https://blackfire.io/profiles/972bb55f-97ad-4870-93be-bfed8f50ad3b/graph (the difference is that we run Fibers and we also store pending Fibers for later when the expr result is available)

Right now it's slower because of these reasons. Maybe some optimizations could make it faster. Feel free to send PRs against fnsr-storage, thanks :)

What I will personally look into is to how take advantage of ExpressionResultStorage to not process AST nodes multiple times, like it's done for BooleanAnd+BooleanOr in MutatingScope, and also for closure bodies.

Also I think we could maybe get rid of these properties in MutatingScope, at least when FiberScope is involved:

	/** @var Type[] */
	private array $resolvedTypes = [];

	/** @var array<string, self> */
	private array $truthyScopes = [];

	/** @var array<string, self> */
	private array $falseyScopes = [];

[ondrejmirtes]

Here's comparison 2.1-x -> fnsr-storage: https://blackfire.io/profiles/compare/baf03475-a664-401f-a0df-0f0fd9b1c17b/graph Here's comparison fnsr-storage -> PHPSTAN_FNSR=1: https://blackfire.io/profiles/compare/baf03475-a664-401f-a0df-0f0fd9b1c17b/graph (big increase in memory, presumably because of storing pending Fibers)

[ondrejmirtes]

The build now should be green without any changes, just consuming a bit more memory on large files. I'm going to merge this, then I'm going to clean up the Generator namespace which is no longer going to be developed, then we're going to try to address the downsides of the updated implementation.

[ondrejmirtes]

Once the build with PHPSTAN_FNSR=1 is green on tests and PHPStan, I'm going to add more jobs to the matrix that test it too.

[staabm]

Also I think we could maybe get rid of these properties in MutatingScope, at least when FiberScope is involved:

so the idea is, since we already have a caching with ExpressionResultStorage in FiberScope context, we no longer need this caching in mutating scope when fibers are in use?

[ondrejmirtes]

Yeah, something like that. Of course I could be wrong. For example I was first investigating excessive memory usage because of ExpressionResultStorage::duplicate() but it was totally wrong. The problem instead was in many Fibers in pendingFibers array and the fix instead was https://github.com/phpstan/phpstan-src/commit/4147d7e0eed5ef65b3ed8701c4a6fefdacb52545.