2016-09-25 21:44 GMT+02:00 John D. Ament <[email protected]>: > On Sun, Sep 25, 2016 at 3:16 PM Romain Manni-Bucau <[email protected]> > wrote: > > > Le 25 sept. 2016 21:10, "John D. Ament" <[email protected]> a écrit > : > > > > > > On Sun, Sep 25, 2016 at 3:07 PM Romain Manni-Bucau < > > [email protected]> > > > wrote: > > > > > > > Le 25 sept. 2016 20:57, "John D. Ament" <[email protected]> a > > écrit > > : > > > > > > > > > > On Sun, Sep 25, 2016 at 12:30 PM Mark Struberg > > <[email protected] > > > > > > > > > > wrote: > > > > > > > > > > > > Basically, one sticking target I see continually is > > > > BeanManagerProvider. > > > > > > We already use CDI.current() internally if it is available (via > > > > > > reflection). > > > > > > So no need to upgrade it just for this feature. > > > > > > > > > > > > > > > > Reflection is inherently slower than direct method calls. Hence > > slows > > > > down > > > > > deltaspike's execution. I'll also note that: > > > > > > > > > > - It implies that we need a wrapper. It would be great if we > didn't. > > > > > - Its second in the list, first is JNDI. JNDI will work generally > > > > > everywhere but SE apps. > > > > > > > > > > > > > > > > > https://github.com/apache/deltaspike/blob/master/ > deltaspike/core/api/src/main/java/org/apache/deltaspike/core/api/provider/ > BeanManagerProvider.java#L224 > > > > > > > > > > > > > Quick note here is CDI.current() is slower by design reflection or > not > > but > > > > speed should be ok if code doesnt abuse of it on all lines of a > request > > > > scoped instance so not sure it is that much a criteria. > > > > > > > > > > Slower than compared to... ? > > > There's no spec mandate that CDI.current() act slower. I would expect > > its > > > under the hood accessing the same threadlocal instance for the > > deployment. > > > > > > > To BeanManagerProvider cause one caches the instance the other needs to > > look it up contextually...there shouldnt be any thread local involved > > there. > > > > Ohhh you mean because its CDI.current().getBeanManager(), the > CDI.current() > part is the slow part? That's where I'm saying under the hood a thread > local should be bound with the CDI instance. > > Yep and probably no for the thread local which would be costly for all the cases where CDI.current() is not used (quite often suprisingly - at least surprisingly for me) and would break apps for some other cases but that's an impl detail which doesnt change much the fact BeanManagerProvider is faster (or comparable) and that this is a light layer anyway so doesn't justify by itself the split of the code, no?
> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > but its because we didn't make a DS version > > > > > > > > > > > > > that was CDI 1.1+ compatible. > > > > > > Nope, ALL our versions since day one are CDI-1.1+ compatible. > > > > > > And we also already make use of a few important features. But > only > > via > > > > > > reflection. > > > > > > > > > > > > > > > > I'll clarify this - we didn't release a DS version that was only > CDI > > 1.1+ > > > > > compatible. We've always carried around the "baggage" of CDI 1.0. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > features like manual injection of fields, which > > > > > > > > > > > > > could be replaced by Unmaanaged. > > > > > > > > > > > > I don't like Unmanaged as it can easily create mem leaks. It is > imo > > as > > > > > > unnecessary as @New used to be... > > > > > > I already expressed my concerns in the EG, but it's too late to > get > > rid > > > > of > > > > > > it now. > > > > > > Also note that Unmanaged always creates a newInstance while the > > > > DeltaSpike > > > > > > utility method injects into a given EXISTING instance. That is a > > *huge* > > > > > > difference. > > > > > > > > > > > > > > > > CDI's pretty funny, its the only spec I can think of that > inherently > > > > > creates memory leaks. Unmanaged shouldn't create memory leaks. > > Maybe > > > > the > > > > > underlying problem is that the impls treat it as a dependent scoped > > bean? > > > > > > > > > > Anyways, most cases I've seen for BeanProvider.injectFields uses > this > > > > > format: > > > > > > > > > > SomeBean someBean = BeanProvider.injectFields(new > > > > SomeBean(someOtherDep)); > > > > > > > > > > E.g. the object isn't really valid until the injection points are > > > > satisfied. > > > > > > > > > > > > > > > > > > > > > > > > > > > > LieGrue, > > > > > > strub > > > > > > > > > > > > > > > > > > > > > > > > > On Sunday, 25 September 2016, 17:37, John D. Ament < > > > > > > [email protected]> wrote: > > > > > > > > On Sun, Sep 25, 2016 at 11:34 AM Thomas Andraschko < > > > > > > > [email protected]> wrote: > > > > > > > > > > > > > >> not sure if a cdi2-module is enough > > > > > > >> we should also get rid of some of our api's which are in CDI > > 2.0 > > > > now > > > > > > >> > > > > > > > > > > > > > > Yes. I agree. Basically, one sticking target I see > continually > > is > > > > > > > BeanManagerProvider. Maybe we keep it around as a utility and > > for > > > > > > > backwards compatibility, but its now available as > CDI.current(), > > to > > > > do > > > > > > > programmatic look up. > > > > > > > > > > > > > > In addition, there are features like manual injection of > fields, > > > > which > > > > > > > could be replaced by Unmaanaged. I know as a user of CDI 1.2, > > seeing > > > > > > both > > > > > > > available makes me confused, but its because we didn't make a > DS > > > > version > > > > > > > that was CDI 1.1+ compatible. > > > > > > > > > > > > > > John > > > > > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > >> 2016-09-25 17:28 GMT+02:00 Romain Manni-Bucau > > > > > > > <[email protected]>: > > > > > > >> > > > > > > >> > 2016-09-25 17:22 GMT+02:00 John D. Ament > > > > > > > <[email protected]>: > > > > > > >> > > > > > > > >> > > Hey guys, > > > > > > >> > > > > > > > > >> > > Since its inception, DeltaSpike has targeted Java EE 6 > and > > > > lower, > > > > > > > and > > > > > > >> as > > > > > > >> > a > > > > > > >> > > result the CDI 1.0 runtime. We have maintained a pretty > > > > > > > backwards > > > > > > >> > > compatible code base for 5 years now. > > > > > > >> > > > > > > > > >> > > CDI 2.0 is going to wrap up in January, if current > > schedules > > > > > > > align > > > > > > >> > > correctly. > > > > > > >> > > > > > > > > >> > > I'd like to propose that we start a branch for 2.0 > > > > > > > development now. It > > > > > > >> > > would be a good place to put fixes for > > > > > > >> > > https://issues.apache.org/jira/browse/DELTASPIKE-1206 > and > > > > other > > > > > > >> > > enhancements that we can make to our core runtime to > better > > > > > > > integrate > > > > > > >> > with > > > > > > >> > > CDI 1.1/1.2/2.0 features that have been added. In > addition > > to > > > > > > > the > > > > > > >> Java 8 > > > > > > >> > > upgrade taking place there. > > > > > > >> > > > > > > > > >> > > We can keep master on 1.x for patches that may be needed > > for > > > > the > > > > > > > 1.x > > > > > > >> > line, > > > > > > >> > > and rebase them with a 2.0 branch to make sure both > > branches > > > > get > > > > > > > the > > > > > > >> > fixes. > > > > > > >> > > > > > > > > >> > > WDYT? > > > > > > >> > > > > > > > > >> > > > > > > > >> > What feature do we target and need CDI 2.0 for it? If none > I > > > > think we > > > > > > >> don't > > > > > > >> > need the branch yet, if enough we should also think to > have a > > > > cdi2 > > > > > > > module > > > > > > >> > to avoid to fork code while 1.0/1.1 is maintained > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > > >> > > John > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > > > > > >
