On 14 Dec 2012, at 20:05, Bill Shannon wrote:
> Pete Muir wrote on 12/14/12 05:28:
>>
>> On 13 Dec 2012, at 23:02, Bill Shannon wrote:
>>
>>> Thanks for putting this together, Pete! I think this is a good compromise
>>> that will help the community. I have some questions and some comments...
>>>
>>>> Only classes with a bean defining annotation, or with an annotation, or
>>>> meta-annotation, present specified by @WithAnnotations are passed to
>>>> ProcessAnnotatedType observers (the exact semantics are defined by CDI
>>>> 1.1 PRD for @WithAnnotations). As mentioned above, if a
>>>> ProcessAnnotatedType is observed for a type without a bean defining
>>>> annotation, as a result of having an annotation present that is specified
>>>> by @WithAnnotations, it may instruct the container to add discover the
>>>> class as a bean.
>>>
>>> I'm trying to understand the impact of this on the implementation.
>>>
>>> If I remember correctly, portable extensions are discovered using
>>> java.util.ServiceLoader, is that correct?
>>
>> Yes.
>>
>>> After discovering the services during CDI initialization, CDI looks for
>>> the @WithAnnotations annotation and notifies the extension of the classes
>>> it's interested in.
>>
>> Yes.
>>
>>> That means that CDI must have at its disposal a list of *all* classes with
>>> any annotation the could ever possibly be specified with @WithAnnotations,
>>> is that right?
>>
>> Right. This would be a container specific integration SPI, that would be
>> defined e.g. by Weld or OWB. I suspect it would be more likely implemented as
>> an external lookup. But like I say, nothing guaranteed.
>>
>>> And even worse, for those portable extensions that don't use
>>> @WithAnnotations, CDI needs to have a list of all classes in the
>>> application, right?
>>
>> In short, no. Only for archives deployed the 1.0 (beans.xml enabled) way. For
>> the CDI 1.1 way (bean defining annotation) you must use @WithAnnotations to
>> receive any classes that don't have a bean defining annotation on them.
>
> But even with CDI 1.1 there will be a way to tell it to use all classes
> as beans, right?
Yes.
> So the container needs to be capable of telling CDI
> about all classes.
Yes, however only if beans.xml exists, with specific piece of XML.
> And to optimize its work in the new common case,
> it would need a way to ask CDI which "mode" it's running in before it
> collects all the information about the classes, and it needs to be able
> to ask this question before CDI is initialized.
This is really an implementation detail, and up to the CDI impl to specify it's SPI.
However, I don't think this is a problem, the bread and butter of a Java EE container is checking if files exist, and parsing XML.
To give you an idea of how the RI does it, Weld 1.1 allows a container to parse beans.xml itself, and parse in a object model representing it's content. Containers often want to parse deployment descriptors themselves, as it allows them to offer an interceptor-like facility around the parsing.
Weld also offers a parse utility (exposed via the Bootstrap class) which can be called at any point, it's not dependent on the CDI container having been initialized at all.
When we specify a bootstrap SPI for CDI, I would expect to offer a similar facility. In short, I think this is a solved problem.
In general, the CDI container integration SPIs are basically non-existent, partly because of JSR-330 promising and failing to deliver a bootstrap model.
>
>>> So even though CDI might define many fewer beans by default, it still needs
>>> to be able to find out about all classes in the application, and in that
>>> respect it's the same as CDI 1.0.
>>
>> No, as per my previous answer. It will only need to classload classes with
>> bean defining annotations or those added by @WithAnnotations. This can be
>> totally done through byte-code scanning, there is no need to classload the
>> classes.
>
> Except that you have to know which mode CDI is operating in in order
> to decide how much or how little work you have to do.
See above.
>
>>> The reduced overhead is not in the class scanning or annotation discovery,
>>> its in the bean initialization, right?
>>
>> The reduced overhead is in not requiring classloading of all classes, instead
>> allowing annotations to be discovered by byte-code scanning. Our analysis
>> suggests that around 80-90% of the initialization time of CDI is spent
>> classloading. If we can restrict what gets classloaded to just that which the
>> user explicitly specifies, this should help a lot. If you used reflection to
>> do your scanning, then yes, this would increase overhead.
>
> Again, which means there needs to be an SPI of some sort for the container
> to ask CDI how much information it needs to collect.
See above.
>
>>>> OPEN ISSUE: Should auto-discover be false by default for beans.xml with
>>>> version 1.1. This would mean that adding a beans.xml would have no
>>>> impact on discovery for 1.1 apps, however it is a significant change from
>>>> 1.0.
>>>
>>> There's some potential cost to developers (in confusion) if you change the
>>> behavior when a beans.xml is present, but if you don't think that's a big
>>> issue the default should probably be the programming model you most want to
>>> encourage.
>>
>> If we were defining this in 1.0, then I would say yes to this question.
>>
>>>
>>>> OPEN ISSUE: Should only scopes for which a CDI context exists be
>>>> considered component defining? This could introduce some thorny edge
>>>> cases, but would address the JSR-330 compatibility issue better.
>>>
>>> How does a CDI context come to exist for a given scope?
>>
>> Either there are built-in ones defined by the spec, or an extension
>> explicitly adds one using an API.
>
> I guess I don't understand how the SPI works here. If an extension
> can add a new context, then you can't collect the annotation information
> until all the extensions execute. But don't you need the annotation
> information to know which extensions to invoke based on their @WithAnnotations
> annotation?
Yes. This isn't about performance, but about other JSR-330 implementations. If we do this, then we don't risk enabling beans for something that intended to use another JSR-330 impl.
>
>>>> OPEN ISSUE: Should we extend auto-discover in beans.xml to allow complete
>>>> disablement of scanning e.g.
>>>> auto-discover="all|bean-defining-annotations-only|none" ?
>>>
>>> This seems reasonable, although I would probably name it something like
>>> "beans=all|annotated|none".
>>
>> Yes, naming is definitely still open.
>>
>>>
>>>> OPEN ISSUE: How should the ProcessAnnotatedType event instruct the
>>>> container to discover a class as a bean? Perhaps something like
>>>> event.discover(clazz)?
>>>
>>> I don't understand this issue.
>>
>> ProcessAnnotatedType may be observed for classes that don't currently have a
>> bean defining annotation, but are picked up due to @WithAnnotations. We want
>> to allow the extension to say "this class should be discovered as a bean".
>> How should we do that?
>
> Is that the same as the issue I mentioned above?
No. This about the SPI CDI extension authors would use. What should it look like? ProcessAnnotatedType.discover(class) for example?
>
>>>> OPEN ISSUE: Should we integrate this with the package level scanning
>>>> control we have proposed for CDI 1.1?
>>>
>>> I'm not familiar with the package level scanning control, but this sounds
>>> like a reasonable approach.
>>
>