Re: Re: PHP Annotations RFC + Patch

Hi Johannes, Comments inline. 2010/9/8 Johannes Schlüter <johannes@schlueters.de>: > Hi, > > On Wed, 2010-09-08 at 13:44 -0300, Guilherme Blanco wrote: >> >>> 2) I suppose that usage of annotation would be quite rare case. I don't >> >>> think it make sense to extend each op_array, property and class with >> >>> additional "annotations" field. I think it's possible to have a >> >>> separate >> >>> CG(annotaions_map) where the keys might be the addresses of corresponding >> >>> classesm, methods and properties. >> >> >> >> Annotations are metadata related to a code element. IMHO it make sense >> >> to add them in the appropriate code element structures (even >> >> doc_comment is in those structures) so that APC and other cache will >> >> know that this is something part of the code element and it needs to >> >> be cached and so on. >> > >> > It's easier to keep it in appropriate places, but it's a waste of memory, >> > because only a few classes will use it. According to doc comments, in case >> > we accept annotations, I would represent it as a special kind of annotation. >> > >> > Separate map(enity -> anotations) would require memory only for defined >> > annotations. Of course it would be a bit more expensive to read it and would >> > require special handling in opcode hashes. >> >> Argument is not reasonable IMHO. >> You may know that most code around web lack of documentation, >> specially docblock, so why not separate the docblocks on a separate >> structure too? >> Sorry, but your argument is not reasonable. >> Each class/property/method should contain all relevant information it >> can get, whatever it needs (docblock, annotations, visibility >> information, etc). Of course you have a bit of memory overhead, but >> it's a pointer, isn't it? So at the end, we will only have overhead >> when we have annotations defined. >> Also, decomposing the annotation definition and storage adds a level >> of dependency and indirection that may get things harder to comprehend >> by newer contributors. > > Let's do some math: > > A pointer on a 64bit architecture is 8 bytes. A random PHP binary of > mine has 1226 functions. And 133 classes. Each of them has, let's say 10 > methods (i guess average is higher, but well). > > Then I'm running an application, which has maybe 100 classes with 10 > methods each. > > This gives 8 * (1226 + 133 + 100 + (133 * 10) + (100 * 10)) bytes = > 30.31 kilobytes. > > Now I have 100 server children, so this are 3.021 megabytes on the > server. > > Oh and that's only for the pointer which _could_ point to annotations > but will most likely point to NULL. > > With the approach proposed by Dmitry this memory won't be used but > whenever annotations are accessed a lookup more will be needed. > > This calculation is of course completely wrong to many wild guesses and > simplifications in there. > > Of course that's not much but your statement shows what will happen > next: > >    "specially docblock, so why not separate the docblocks" > > First of all I claim that there are more docblocks than annotations. > > Secondly what about the next pointer, and the next and the next somebody > wants to add, not only in the op array but other global structures, one > pointer in the zval, one in the hash bucket, one ...? > You are right. I was thinking strictly to code coupling and containers by responsability. I have implemented the support under docblock parsing for Doctrine 2, and we cache the parsed pieces under the name of ClassMetadata. That's why I don't feel too much about having it standalone or coupled per associated piece. I think related to this argument I missed a little the "eat your own dog food", mainly because I've never done too much PHP internals. Anyway, I feel a little uncomfortable when I see localized code using global resources, it sounds a little like bad architecturing. But I completely understand the performance reasons. =) > We can still move this over later when annotations are adopted and the > equation memory vs. CPU for this feature is valued differently. > I didn't mean to be rude. I've re-read my email and it really sounded like harsh... sorry! I'm just very anxious to see this merged on core... I've bothered people 2+ years ago to see this alive on 5.3 and now it's back to discussion and more close to become reality... I just wanna get things right and "in". =) > johannes > > -- Guilherme Blanco Mobile: +55 (16) 9215-8480 MSN: guilhermeblanco@hotmail.com São Paulo - SP/Brazil