OpenWebBeans
  1. OpenWebBeans
  2. OWB-619

@New beans must only exist if there is at least one injection point for them

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.1
    • Fix Version/s: 1.1.2
    • Component/s: Core
    • Labels:
      None

      Description

      spec section 3.12 @New defines

      >For each managed bean, and for each session bean, a second bean exists which ....
      ...
      > is enabled, in the sense of Section 5.1.2, "Enabled and disabled beans", if and only if some other enabled bean has an injection point with the qualifier @New(X.class) where X is the bean class.

      Which means that we dont need to create a NewBean for each and every bean, but only for ones which gets used in an InjectionPoint somewhere!
      This can massively reduce the amount of needed beans and the mem consumption of OWB

        Activity

        Hide
        Mark Struberg added a comment -

        shipped in OpenWebBeans-1.1.2

        Show
        Mark Struberg added a comment - shipped in OpenWebBeans-1.1.2
        Hide
        Mark Struberg added a comment -

        Talked with other EG folks. We really only need to/must create a @NewBean if it is used in an injectionpoint, but not for each and every bean around.

        Our internal mem consumption dropped drastically since we only need to store half the number of Beans now...

        Show
        Mark Struberg added a comment - Talked with other EG folks. We really only need to/must create a @NewBean if it is used in an injectionpoint, but not for each and every bean around. Our internal mem consumption dropped drastically since we only need to store half the number of Beans now...
        Hide
        Arne Limburg added a comment -

        Section 5.1.2 talk about enabled and disabled beans. All the disabled beans it talks about are available via BeanManager.getBeans(...)

        Show
        Arne Limburg added a comment - Section 5.1.2 talk about enabled and disabled beans. All the disabled beans it talks about are available via BeanManager.getBeans(...)
        Hide
        Mark Struberg added a comment -

        Actually Bean<T> nor Contextual<T> have any isEnabled(). Thus I guess what is meant by the spec is that they are just not available in the BeanManager.

        See 11.1. The Bean interface
        > An instance of Bean exists for every enabled bean.

        Show
        Mark Struberg added a comment - Actually Bean<T> nor Contextual<T> have any isEnabled(). Thus I guess what is meant by the spec is that they are just not available in the BeanManager. See 11.1. The Bean interface > An instance of Bean exists for every enabled bean.
        Hide
        Arne Limburg added a comment -

        The spec says that this bean exists, so it should be returned by BeanManager.getBeans(...)
        However if no injection point exists, it is not active so it should be filtered out by BeanManager.resolve(...)
        So imho the creation of such bean does not depend on the existence of an injection point.
        Just the result of BeanManager.resolve(...) does.

        Show
        Arne Limburg added a comment - The spec says that this bean exists, so it should be returned by BeanManager.getBeans(...) However if no injection point exists, it is not active so it should be filtered out by BeanManager.resolve(...) So imho the creation of such bean does not depend on the existence of an injection point. Just the result of BeanManager.resolve(...) does.
        Hide
        Mark Struberg added a comment -

        Hmm, it changes the behaviour of course. But it would then be spec conform, so I'd rather say it would 'fix' the code rather than break it

        Show
        Mark Struberg added a comment - Hmm, it changes the behaviour of course. But it would then be spec conform, so I'd rather say it would 'fix' the code rather than break it
        Hide
        Arne Limburg added a comment -

        I just wanted to point out that your suggested change may break existing code.

        Show
        Arne Limburg added a comment - I just wanted to point out that your suggested change may break existing code.
        Hide
        Arne Limburg added a comment -

        I am perfectly fine with not creating a NewBean for every managed Bean. Let's discuss the implementation details on the dev-list

        Show
        Arne Limburg added a comment - I am perfectly fine with not creating a NewBean for every managed Bean. Let's discuss the implementation details on the dev-list
        Hide
        Mark Struberg added a comment -

        I agree that it's not very user friendly, but honestly: did you ever use @New ?
        It's practically irrelevant, because it is only intended for the @Inject @New(MyClass.class) use case. There is no dynamic nor whatever lookup allowed by the spec atm. You cannot even trigger producer methods or fields with it...

        Show
        Mark Struberg added a comment - I agree that it's not very user friendly, but honestly: did you ever use @New ? It's practically irrelevant, because it is only intended for the @Inject @New(MyClass.class) use case. There is no dynamic nor whatever lookup allowed by the spec atm. You cannot even trigger producer methods or fields with it...
        Hide
        Arne Limburg added a comment -

        This is another point where the specified behavior is not obvious to the user. This means, that a user cannot predict, if getBeans(Bla.class, newLiteral) will work or not, since at that point he/she cannot know if there exists such injection point.

        Dynamic creation of that bean would be the better solution here, imho. But I guess I have to raise a spec issue for this. At least it should be specified that the @New literal is forbitten in general as parameter for getBeans(...)

        Show
        Arne Limburg added a comment - This is another point where the specified behavior is not obvious to the user. This means, that a user cannot predict, if getBeans(Bla.class, newLiteral) will work or not, since at that point he/she cannot know if there exists such injection point. Dynamic creation of that bean would be the better solution here, imho. But I guess I have to raise a spec issue for this. At least it should be specified that the @New literal is forbitten in general as parameter for getBeans(...)
        Hide
        Mark Struberg added a comment -

        thats exactly what I asked jboss folks too. Weld is doing the same. In fact, they just added this section pretty late because they had performance problems. Doing a getBeans(Bla.class, newLiteral) is not permitted if Bla doesnt get used at an injection point! This is how the RI behaves too.

        Show
        Mark Struberg added a comment - thats exactly what I asked jboss folks too. Weld is doing the same. In fact, they just added this section pretty late because they had performance problems. Doing a getBeans(Bla.class, newLiteral) is not permitted if Bla doesnt get used at an injection point! This is how the RI behaves too.
        Hide
        Arne Limburg added a comment -

        From reading the spec you are right. But I am wondering, if this change would break existing code, since @New is a qualifier, it could be used in BeanManager.getBeans or Instance.select... which would change behavior (i.e. not return any bean) then.

        Should we add a special handling for the @New qualifier in the injection resolver, too?

        Show
        Arne Limburg added a comment - From reading the spec you are right. But I am wondering, if this change would break existing code, since @New is a qualifier, it could be used in BeanManager.getBeans or Instance.select... which would change behavior (i.e. not return any bean) then. Should we add a special handling for the @New qualifier in the injection resolver, too?

          People

          • Assignee:
            Mark Struberg
            Reporter:
            Mark Struberg
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development