Reflecting same class trees leads to extremely high amount of repeated parsing/AST traversal and therefore overhead

[Ocramius]

To reproduce, something like this would probably suffice:

$sourceLocator = new StringSourceLocator(
<<<'PHP'
<?php

class A {}
class B extends A {}
class C extends A {}
PHP
    ,
    (new BetterReflection())->astLocator()
);

$reflector = new ClassReflector($sourceLocator);

// now put a breakpoint in `ClassReflector#reflect()` and notice that it is being called 4 times instead of 3:

$b = $reflector->reflect('B');
$b->getParentClassNames();
$c = $reflector->reflect('C');
$c->getParentClassNames();

This is because the various internal components don't cache the reflections.

What we can do:

• make the ClassReflector and the FunctionReflector keep an internal cache by default (not so nice)
• make a CachedReflector that caches based on the requested identifier (not so easy due to delegation logic happening in the ClassReflector internals)

[Ocramius]

Removing from the 3.0.0 milestone.

This is a hard to crack problem, and 90%+ of the overhead comes from this bit:

BetterReflection/src/SourceLocator/Ast/FindReflectionsInTree.php

Lines 127 to 130 in f8b3fc6

	$nodeTraverser = new NodeTraverser();
	$nodeTraverser->addVisitor(new NameResolver());
	$nodeTraverser->addVisitor($nodeVisitor);
	$nodeTraverser->traverse($ast);

Possible approaches are perform that iteration only once during parsing and memoizing (as part of the "located source" operations) but that's not in our current scope.

[Ocramius]

To clarify further, as per #419:

I think we'll need to have a radically different approach:

1. parsing should happen before the SourceLocator is asked for symbols
2. store the parsed AST in the LocatedSource instances (memoization). In addition to that, we need to store following in the LocatedSource:
   • the normalized AST (passed through the NameResolver)
   • the AST for classes/functions and constants being discovered (what FindReflectionsInTree is currently doing)

This can be done lazily when LocatedSource is queried for identifiers, but if we do it once per SourceLocator instance, we're avoiding repeated processing that cannot be avoided.

[sebastianbergmann]

I would like to use BetterReflection in the part of PHPUnit that finds the tests (instead of requireing *Test.php files and then using the built-in ReflectionClass on each class). I have something that works but it is slow (which is not unexpected).

Implementing my own caching on a file level is rather trivial ... but it falls short when inheritance comes into play. Is this something you are considering for your caching? What I mean is tracking that the information B needs to be updated when A changes for class A {} class B extends A {}.

[Ocramius]

I think this is largely solved by #461, which makes each lookup extremely fast, and found the root problem of most performance issues so far.

Before that, each class lookup (including already found symbols) would lead to a SourceLocator traversal.

[Ocramius]

Closing: this is indeed handled by #461.

This is strictly related to #612 and #614 too.

The problem itself is that each source locator has a "strategy" for finding symbols, and some locators are just "dumb", and should probably stay dumb. Implementing custom/smarter locators has been done in #461 (for composer-friendly projects), and changing the pre-existing ones is not necessary, and will only lead to potential regressions.