This content has been marked as final.
Show 32 replies
-
30. Re: integration with the Papaki annotation indexer/repositor
dmlloyd Oct 29, 2009 10:13 AM (in response to smarlow)"alesj" wrote:
"emuckenhuber" wrote:
* jboss-metadata - for example jboss-metadata is just asking the AE to get a list of classes which have a given top-level annotation (e.g. @Stateful). Then loading the class and do a separate annotation processing with java reflect. This is also because AE does not have enough information about inheritance, which is needed for all the EE annotation processing.
But then you should not use java reflect, but JBoss Reflect. ;-)
Why?
Because this info is already present if we use McAnn, as McAnn already uses JBoss Reflect, and Reflect caches this info.
And this also shows that things go hand in hand, and that this inheritance check should not be part of annotation scanning.
Not to mention another very good example why re-using existing libs is good. ;-)
I come to the opposite conclusion. The ideal indexer will, in my opinion, scan a set of classes associated with an archive once (at run time), and maintain a completely string-based index of annotations -> members (by name), and classes -> superclasses (by name). With this information, and one index per archive, you can find any annotation (including inherited) on any member of any class without loading the class or using reflection in any way. If you were to use an optimized string map like a trie for example, it's possible this data could be collected very quickly and stored in much less space than the equivalent reflective solution.
In addition, it's possible that this preindexing can be used to accelerate classloading, at least to the extent that missing classes can be identified without querying the filesystem.
Finally, using this system one can reuse the same index even when the same class or JAR is part of many classloaders, since the index is based only on the file structure and not on what's been loaded (think of shared JARs in the classpath of more than one isolated EAR, for example).
One of the driving forces behind the discussion was the need (in EJB3, IIRC) to be able to perform this kind of scanning without loading classes at all, even in the presence of inheritance across modules. And this is a case where the existing solutions completely fail to meet the requirements.
This is in addition to the use case of pre-indexing; but once such a fast/efficient index exists for online scanning, why not reuse it for offline scanning too?
And this, as one might say, is why reusing existing libs is bad. :-) At least, using existing libs even when they don't quite meet the requirements. -
31. Re: integration with the Papaki annotation indexer/repositor
alesj Oct 29, 2009 10:48 AM (in response to smarlow)"david.lloyd@jboss.com" wrote:
I come to the opposite conclusion. The ideal indexer will, in my opinion, scan a set of classes associated with an archive once (at run time), and maintain a completely string-based index of annotations -> members (by name), and classes -> superclasses (by name). With this information, and one index per archive, you can find any annotation (including inherited) on any member of any class without loading the class or using reflection in any way. If you were to use an optimized string map like a trie for example, it's possible this data could be collected very quickly and stored in much less space than the equivalent reflective solution.
This is one way of doing it.
But the fact is that you'll end up loading most of the stuff anyway,
so I don't see too much value with this approach vs. lazy reflective approach."david.lloyd@jboss.com" wrote:
In addition, it's possible that this preindexing can be used to accelerate classloading, at least to the extent that missing classes can be identified without querying the filesystem.
I wouldn't mix the two of them.
CL is complex enough w/o this. ;-)"david.lloyd@jboss.com" wrote:
Finally, using this system one can reuse the same index even when the same class or JAR is part of many classloaders, since the index is based only on the file structure and not on what's been loaded (think of shared JARs in the classpath of more than one isolated EAR, for example).
I don't see how or why this helps?
The actual info will always be minimal - first loaded class / ctclass.
The rest is to mimic ann-repo according to CL hierarchy, which is where this reuse comes in play."david.lloyd@jboss.com" wrote:
One of the driving forces behind the discussion was the need (in EJB3, IIRC) to be able to perform this kind of scanning without loading classes at all, even in the presence of inheritance across modules. And this is a case where the existing solutions completely fail to meet the requirements.
Why or where did you pick up this fact?
If we do scanning with Javassist based Reflect, we don't load anything until it's absolutely necessary."david.lloyd@jboss.com" wrote:
This is in addition to the use case of pre-indexing; but once such a fast/efficient index exists for online scanning, why not reuse it for offline scanning too?
That's exactly what we're doing.
Did you even bother to look at the project(s)?"david.lloyd@jboss.com" wrote:
And this, as one might say, is why reusing existing libs is bad. :-) At least, using existing libs even when they don't quite meet the requirements.
Again, which missing requirements are we talking about here?
As much as I appreciate you pushing the ideas, arguments, ...
it still seems to me that you're arguing here with very little known facts,
which means you're consequently wasting my and others time. -
32. Re: integration with the Papaki annotation indexer/repositor
emuckenhuber Oct 29, 2009 11:59 AM (in response to smarlow)"alesj" wrote:
"emuckenhuber" wrote:
* jboss-metadata - for example jboss-metadata is just asking the AE to get a list of classes which have a given top-level annotation (e.g. @Stateful). Then loading the class and do a separate annotation processing with java reflect. This is also because AE does not have enough information about inheritance, which is needed for all the EE annotation processing.
But then you should not use java reflect, but JBoss Reflect. ;-)
Why?
Because this info is already present if we use McAnn, as McAnn already uses JBoss Reflect, and Reflect caches this info.
And this also shows that things go hand in hand, and that this inheritance check should not be part of annotation scanning.
I was talking about where the problems are and not what should be done in the end.
Since this seems to be a discussion about the requirements of a annotation scanning API - we might want to think of an easy to use API which covers 80% of the use cases. I agree that JBoss Reflect should be used instead of java reflect, but only in special cases.
I still think that the class hierachy should be in the annotation API. Whether reflect is caching stuff or not, it is not using the index to avoid duplicate work?"alesj" wrote:
"emuckenhuber" wrote:
Beside jboss-metadata there are not many users of AnnotationEnvironment, but we really should avoid having things like getClassloader().loadClass(metaData.getClassName()).getAnnotation(X) if possible.
Sure, leave it to JBoss Reflect. It's already there.
And with Reflect running on top of Javassist it's gonna be lazy as possible.
As said the problem is not jboss-metadata alone - other deployers are doing the same."alesj" wrote:
"emuckenhuber" wrote:
* jboss-mdr integration - this index should be reused in the MDR as well, as mentioned earlier in this thread.
Yes.
But this is just a matter of properly populating MDR's MetaDataLoaders.
e.g. sort of an MetaDataLoaderToAEAdapter
AFAIK it is not done yet, that's why i listed it here."alesj" wrote:
"emuckenhuber" wrote:
* inclusion/exclusion of deployments, .jars, classes - we need a way to exclude stuff from annotation scanning, as we are mostly just doing too much scanning. Basically we already have jboss-scanning.xml - which makes sense for our own deployments. Still we need a programmatic way of excluding stuff like:
- if the deployment is not an EE5/EE6 deployment we can just skip any annotation scanning,
- if isMetaDataComplete()
- afaik the new servlet spec requires to exclude .jars (web-fragments), where we should be able to skip scanning as well.
This is all already there, in Deployers.
Either as part of ScanningMetaData or AnnotationScanner's DeploymentUnit filters.
* e.g. http://anonsvn.jboss.org/repos/jbossas/branches/Branch_5_x/server/src/main/org/jboss/deployment/JBossMetaDataDeploymentUnitFilter.java
Web deployers should just properly push required info about fragments to this mechanisms.
e.g. using ScanningMetaDataBuilder (which you promised to write ;-)
First there should be no need for this DeploymentUnitFilter.
I agree with David suggesting that the annotations should get pushed to the index. So AE should be lazy, only doing stuff when it's called by the deployers. Then we could get rid of a separate filter implementation for each meta data type.
At least for jboss-metadata the following two methods should be enough (well as far as i can recall) as long as there are context information like AnnotatedInfo with declaringClass, method signature and similar stuff - but i think that's already there.1) AE.getClassesWithTypeAnnotation(Stateless.class); and 2) AE.getAnnotationsForClass("org.jboss.test.MyStatelesBean"); maybe for completeness: 3) AE.getAnnotation(String clazzName, Annotation annotation);
Which basically means that 1) tells AE to scan the deploymentUnit classpath and 2) tells AE to recursively scan the class (this should also take annotation outside of the deploymentUnit into account). Of course all operations should be matched against the index, and if it's not there it has to be scanned at runtime.
Maybe there is even a need to override the ScanningMetaData within the deployer - so that web can exclude libraries, but a 2nd deployer e.g. BeanMetaDataDeployer (or whatever) would need to get the unfiltered DeploymentUnit - so this library would be partially added to the index (if it's not already there...)