Pete Muir wrote on 12/14/12 13:15:
>
> 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.
I agree. And if you want to move this discussion off this list that would
be fine with me. :-)
> 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.
I definitely wouldn't want both GlassFish code and Weld code parsing
beans.xml and making independent decisions about whether or not CDI
is enabled and in what mode.
> 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.
This is more than just parsing, this is about answering the question of
which classes need to be scanned.
> 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.
I understand, and when we finally standardize SPIs in this area it will
be even more important that we properly divide the functionality between
the container and the CDI implementation.
>>>> 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.
Supporting other JSR-330 implementations in a Java EE application is far
from a primary use case. I'm ok if such applications have to explicitly
disable CDI.
>>>>> 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?
I don't know enough about the CDI SPI to help with that... :-(