Added a PSR-0 Annotation Reader

[chx]

You need to pass a PSR-0 include path and a class name to this class and then you can get the annotations out of it. The goal of this exercise is to converse memory as much as possible. Reflection requires the class to be loaded and can not be unloaded. Tokenizing allows for unloading. There was already groundwork for tokenizing for the use statements so I have patched the PhpParser class to allow reuse.

[stof]

unset will unset the properties of the class, not the values of the properties

[beberlei]

I don't get what this has to do with PSR-0. What it does is using tokenizer instead of reflection, alkso its a parser and not a reader. This PR is really weird to be honest in breaking the naming and hierachy :-) I dont see the value here.

[schmittjoh]

PSR-0 probably relates to the fact that the class is expected in a specific filename, and he does not rely on an autoloader to find it. I can see why this might be useful sometimes. I haven't tested performance, but I guess it will be slower the larger the class becomes (token_get_all hasn't a really good record there).

If we add this, I think we need to change the interface to remove the reflection methods (probably adding a second interface). Also extending the PHPParser and duplicating a lot of code does not really look good. We should try to avoid that. Maybe introducing a doc comment provider which can be injected into the reader and is either reflection-based or uses token_get_all.

[akkie]

The first version of the PhpParser was horrible slow when tokenizing the whole file. As I see in your pull request you make the same mistake and tokenize the whole file.

Please look at this benchmark:
https://github.com/akkie/parser-benchmark

[schmittjoh]

Whatever we do, the performance of the Reflection-based solution should not deteriorate.

[beberlei]

I don't see why we need different implementations here. The way it is at the moment works, why do we need to make this difficult for the users by adding a decision?

[schmittjoh]

One thing where I could need it, is static analysis. I do not want to execute user-provided code. @chx apparently wants to create less classes (i.e. saving memory) which seems another use-case.

We can still make the default decision for the user but allow to inject a different doc comment provider.

[neclimdul]

@akkie that's an interested benchmark but it doesn't test the performance of the Reader methods here, just the class parsing. Any ideas how we could compare that?

@beberlei I think the idea is that the tokenizer doesn't take a reflection class so the code is not permanently loaded into memory. It could be a fairly novel and useful choice.

[sdboyer]

it's not so much about the reflection classes themselves consuming memory as it is about the parsed-in code consuming memory. without an opcode cache (which we can't assume/require with Drupal), there's a memory hit for every file that's parsed in. userspace code can't unload code once it's been parsed in, so there's no way to recoup the memory later. the goal of taking a pure-tokenized approach was saving memory by avoiding parsing in in the first place. and since the large token array is flushed by the end of the constructor, the objects are as thin on memory use as possible.

hopefully that clarifies some of the points raised - for example, @stof 's point that this violates the interface. yep, it does, because the whole interface is predicated on Reflection* objects.

@akkie - wow, those numbers are pretty stark. haven't had a chance to review the differences in the PhpParser in detail there, but that's ...heh, noteworthy.

this is starting to look like a case that can be optimized either for speed or memory. both of these seem like reasonable cases to optimize for, especially if the interface could be refactored so that client code can choose (with a simple flag) which optimization they'd prefer to use.

[stof]

@sdboyer as it violates the interface, it cannot implement it and so cannot be used in places expecting the reader (and cannot be wrapped in a cached reader and so on)

[schmittjoh]

We can change the interface (add a new one). That should not be the problem here.

In general, the code is not ready for a merge though.

[beberlei]

well, why not add a static reflection API that looks that the normal API and remove the type-hints?

[chx]

Some notes in random order: the PSR-0 restriction is because I do not want to rewrite PHP in PHP :) I can know from a class name where the file is. I know that there is only one class. Especially without the latter, using the tokenizer would be tricky and less useful. Also, everyone uses PSR-0 by now anyways, so why not rely on it?

There is some code duplication here for sure but not a real lot. If I manage to unify the interfaces then I can move the globalIgnoredNames handling into a base class (I think late static binding will be useful) and also the getAnnotation function which even the existing AnnotationReader would benefit from.

The unificiation is a bit challenging because for my reader specifying the class makes no sense and getClassAnnotation($class, $annotationName) requires one. Unfortunate :/

[chx]

We will not remove any type hints. Rather I will create a class which extends ReflectionClass and ReflectionMethod and add methods returning such objects to the new parser. So approximately:

$parser = new Psr0Parser('include/path', 'my\name\space\class');
$reader = new AnnotationReader();
$reader->getClassAnnotation($parser->getClassReflection(), $annotationName);

[chx]

Better approach at #154