deltaspike-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gerhard Petracek <gpetra...@apache.org>
Subject Re: [DISCUSS] Start a branch for DS 2.0?
Date Sun, 25 Sep 2016 20:05:58 GMT
-1 for a cdi2 module (we can't improve our api in such a case - e.g. for a
better user-experience in combination with java8+)

+1 for a branch for supporting cdi 1.0+ (= current master) + keeping it
compatible with new spec.-revisions as long as possible/useful.
+1 for moving to ds2 in our master (+ for dropping every module and
workaround we don't need any longer + let's check if the api needs changes
to get a great user-experience in combination with java8+)

regards,
gerhard



2016-09-25 21:56 GMT+02:00 Romain Manni-Bucau <rmannibucau@gmail.com>:

> 2016-09-25 21:44 GMT+02:00 John D. Ament <johndament@apache.org>:
>
> > On Sun, Sep 25, 2016 at 3:16 PM Romain Manni-Bucau <
> rmannibucau@gmail.com>
> > wrote:
> >
> > > Le 25 sept. 2016 21:10, "John D. Ament" <johndament@apache.org> a
> écrit
> > :
> > > >
> > > > On Sun, Sep 25, 2016 at 3:07 PM Romain Manni-Bucau <
> > > rmannibucau@gmail.com>
> > > > wrote:
> > > >
> > > > > Le 25 sept. 2016 20:57, "John D. Ament" <johndament@apache.org>
a
> > > écrit
> > > :
> > > > > >
> > > > > > On Sun, Sep 25, 2016 at 12:30 PM Mark Struberg
> > > <struberg@yahoo.de.invalid
> > > > > >
> > > > > > 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
<
> > > > > > > johndament@apache.org> wrote:
> > > > > > > > > On Sun, Sep 25, 2016 at 11:34 AM Thomas Andraschko
<
> > > > > > > > andraschko.thomas@gmail.com> 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
> > > > > > > > <rmannibucau@gmail.com>:
> > > > > > > >>
> > > > > > > >>  > 2016-09-25 17:22 GMT+02:00 John D. Ament
> > > > > > > > <johndament@apache.org>:
> > > > > > > >>  >
> > > > > > > >>  > > 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
> > > > > > > >>  > >
> > > > > > > >>  >
> > > > > > > >>
> > > > > > > >
> > > > > > >
> > > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message