On 15 Dec 2012, at 00:45, Bill Shannon wrote:
> 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. :-)
Happy either way.
>
>> 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.
Only one or the other would do the parse. Agree that having both do it would be bad.
>
>> 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.
Understood, this was to explain how Weld allows the container to do a parse of beans.xml before Weld is started, and I hope addresses your previous comment about the parsing being done differently. The point is that Weld allows the container to parse beans.xml at any time (no dependencies on Weld starting), examine the metadata (and in 1.1 we can easily have it expose an enum about what level of scanning is required by beans.xml). The container can then hold the result of this, and pass it to Weld when it starts, meaning that the parse isn't repeated.
Obviously the container knows whether beans.xml is found or not (it has to pass it to Weld today), and combined with the parsed beans.xml, can make a decision about what to scan (this will be a ~10 line if statement).
>
>> 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... :-(