1 2 3 Previous Next 33 Replies Latest reply on May 14, 2009 6:07 AM by wolfc Go to original post
      • 15. Re: DESCRIBE phase - Dependency builders for MC Beans
        jaikiran

         

        "alesj" wrote:

        As you can see, this needs an explicit flag in order to check the MD,
        hence it should be rarely true --> checked.

        I missed that :)

        I'll give these changes a try today and let you know how it goes.

        • 16. Re: DESCRIBE phase - Dependency builders for MC Beans
          jaikiran

          While i am trying these things out i'll also check, in the current AS, how many beans need this AOPDependencyBuilder. Just want to reduce the number of MC clients that need to add this additional configuration to override the behaviour.

          • 17. Re: DESCRIBE phase - Dependency builders for MC Beans
            alesj

            I'm just adding tests for this and I stumbled upon a hurdle. :-)

            Having non-AOP DB doesn't mean we'll use non-AOP ConstructorJoinPoint.
            And this could lead to weird behavior; aspect being applied w/o dependencies being properly resolved.

            I'll check how I can tie the two things together.

            • 18. Re: DESCRIBE phase - Dependency builders for MC Beans
              kabirkhan

              Yes, I think AOPConstructorJoinPoint without the AOPDependencyBuilder is pointless, and vice versa.

              It would cause problems if the aspects actually have dependencies

              <aop:aspect xmlns:aop="urn:jboss:aop-beans:1.0"
               name="TxAdvice"
               class="org.acme.aspects.TxAspect"
               <property name="txManager"><inject bean="TxManager"/></property>
               </aop:aspect>
               <aop:bind xmlns:aop="urn:jboss:aop-beans:1.0"
               pointcut="execution(* *->@org.acme.Tx(..))">;
               <advice aspect="TxAdvice" name="invoke"/>
               </aop:bind>
              

              but not really of they don't
              <aop:aspect xmlns:aop="urn:jboss:aop-beans:1.0"
               name="Blah"
               class="org.acme.aspects.Blah"
               </aop:aspect>
               <aop:bind xmlns:aop="urn:jboss:aop-beans:1.0"
               pointcut="execution(* *->@org.acme.Blah(..))">;
               <advice aspect="Blah" name="invoke"/>
               </aop:bind>
              


              But if you are sure you don't want aop for a set of beans, then both should be turned off

              • 19. Re: DESCRIBE phase - Dependency builders for MC Beans
                alesj

                The question is how to force this?
                I don't see any good way.

                But otoh if we leave users such freedom, some mechanism should be in place.
                I wrote the stuff and I got it wrong, no question about it that the users will/would too. :-)

                Another question is where to keep this info, so we don't have to do multiple lookups;
                e.g. in DescribeAction, InstantiateAction, ...

                • 20. Re: DESCRIBE phase - Dependency builders for MC Beans

                   

                  "jaikiran" wrote:
                  While i am trying these things out i'll also check, in the current AS, how many beans need this AOPDependencyBuilder. Just want to reduce the number of MC clients that need to add this additional configuration to override the behaviour.


                  How would you know? The idea of the AOPDependencyBuilder is to make
                  a bean depend upon an aspect. Where only AOP knows that it applies by
                  the looking at its configuration (which is user configurable).

                  Sure there's not much configured by default except the @JMX aspect
                  but that doesn't mean that users won't configure aspects (including for our beans).

                  Its the very nature of aspects being cross cutting that makes the AOPDependencyBuilder
                  required in the first place. i.e. you have to ask aop whether there are any aspects
                  that need to be applied to this pojo and therefore we need to wait for that aspect to be
                  deployed (which might in turn depend upon other things) before deploying the pojo.

                  Otherwise you could just configure the dependency directly on the bean.

                  Just removing the AOPDependencyBuilder might make it faster. But all it will
                  mean is that it will fail later when AOP tries to apply an aspect that is not fully deployed yet.

                  If you really want to exclude the AOPDependencyBuilder processing from a bean then
                  you should really use one of AOP's existing mechanisms like exclude "org.jboss.ejb3.*"
                  from having aspects applied to it (that can be configured on the AspectManager).

                  That way AOP should be able to "short circuit" the aspect dependency checking
                  for those pojos using the same "filters".

                  You'd be better off speaking to Kabir about how to optimise/avoid the AOP checking
                  for what you want than getting Ales to hack things into the MC that are fundamentally wrong.

                  • 21. Re: DESCRIBE phase - Dependency builders for MC Beans
                    kabirkhan

                     

                    "alesj" wrote:
                    The question is how to force this?
                    I don't see any good way.

                    But otoh if we leave users such freedom, some mechanism should be in place.
                    I wrote the stuff and I got it wrong, no question about it that the users will/would too. :-)


                    Maybe have more fields in your annotation, and validate the combinations during one of the early phases (PreInstall or Describe)?

                    "alesj" wrote:

                    Another question is where to keep this info, so we don't have to do multiple lookups;
                    e.g. in DescribeAction, InstantiateAction, ...

                    How costly is an MDR lookup? Compared to the zillion times it is currently hit at the moment to do the pointcut matching, it should be negligible.

                    • 22. Re: DESCRIBE phase - Dependency builders for MC Beans
                      alesj

                       

                      "adrian@jboss.org" wrote:

                      You'd be better off speaking to Kabir about how to optimise/avoid the AOP checking
                      for what you want than getting Ales to hack things into the MC that are fundamentally wrong.

                      I guess this means I can cleanup my hack. :-)

                      • 23. Re: DESCRIBE phase - Dependency builders for MC Beans
                        jaikiran

                         

                        "adrian@jboss.org" wrote:

                        If you really want to exclude the AOPDependencyBuilder processing from a bean then
                        you should really use one of AOP's existing mechanisms like exclude "org.jboss.ejb3.*"
                        from having aspects applied to it (that can be configured on the AspectManager).

                        That's the hack that i tried when i first saw this behaviour :) And that's what showed me an improvement in the deployment timing.

                        "adrian@jboss.org" wrote:

                        Otherwise you could just configure the dependency directly on the bean.

                        That's what we currently do for EJB3 containers.

                        "adrian@jboss.org" wrote:

                        You'd be better off speaking to Kabir about how to optimise/avoid the AOP checking
                        for what you want than getting Ales to hack things into the MC that are fundamentally wrong.

                        Sure, i don't mind trying out a different approach - whichever is the correct one :)

                        • 24. Re: DESCRIBE phase - Dependency builders for MC Beans
                          kabirkhan

                           

                          "adrian@jboss.org" wrote:

                          If you really want to exclude the AOPDependencyBuilder processing from a bean then
                          you should really use one of AOP's existing mechanisms like exclude "org.jboss.ejb3.*"
                          from having aspects applied to it (that can be configured on the AspectManager).

                          That way AOP should be able to "short circuit" the aspect dependency checking
                          for those pojos using the same "filters".


                          Adding something to AOP to avoid the aspect dependency checking in the Describe phase and avoiding the proxy checking in the Instantiate phase is trivial. The question is the easiest/best mechanism to do so.

                          I think the default should be to use AOP as current unless you opt out for a particular bean, since a lot of tests already rely on AOP being turned on. The current filters are classname based, which is not a good choice for this. Although jaikiran says otherwise, AFAICR the filters are turned off for proxies anyway, they only apply to weaving classes.

                          The simplest option would be to add an annotation per bean instance, e.g. @NoAopCheck which would make the AOPDependencyBuilder a no-op, and force the AOPConstructorJoinPoint to avoid the heavy stuff and simply call the default implemetation in its super class BasicConstructorJoinPoint. So, AOP is either on (default) or off for a bean.

                          As you say, this runs the risk of the class not getting the necessary aspects applied to beans where the aop stuff is turned off, but I don't see anything wrong with having this as a choice for an advanced user who knows what they are doing? You have to add the @JMX annotation anyway to register things in JMX, which seems to be the main use-case.

                          • 25. Re: DESCRIBE phase - Dependency builders for MC Beans
                            jaikiran

                             

                            "kabir.khan@jboss.com" wrote:
                            Although jaikiran says otherwise, AFAICR the filters are turned off for proxies anyway, they only apply to weaving classes.


                            I think there's a misunderstanding. When i said the EJB3 containers being deployed as MC beans, i meant the org.jboss.ejb3.EJBContainer and other such EJBTHREE project specific MC Beans and not the proxies that are generated for the end user EJB3 application deployments.



                            • 26. Re: DESCRIBE phase - Dependency builders for MC Beans
                              jason.greene

                               

                              "alesj" wrote:
                              "jaikiran" wrote:
                              But in the long term, shouldn't we be moving away from this default?

                              No, that's why it's called *transparent* AOP integration. ;-)

                              You can only argue that the AOP mechanism is not performant enough,
                              but that's as far as I agree with you.
                              Although this comes with a cost, it does bring lots of nice features.
                              They are mostly true in a sense of AOP; aspects, concerns separation, ...
                              @JMX, @JNDI, @Password, @(add-your-own-metadata-here) are good examples of this.
                              And if I was a user, that's what I would want too, out-of-the-box.


                              It can be the greatest feature in the world, but if the performance sucks as bad as this does, no one will use it. In general we should avoid expanding our usage of this until we know that the design is even fixable.

                              I do agree that supporting various service annotations is useful, and there is even some value in having pluggable behavior. However, building this behavior using arbitrary pointcut expressions in arbitrary locations is beyond overkill.

                              • 27. Re: DESCRIBE phase - Dependency builders for MC Beans
                                alesj

                                 

                                "kabir.khan@jboss.com" wrote:
                                "adrian@jboss.org" wrote:

                                If you really want to exclude the AOPDependencyBuilder processing from a bean then
                                you should really use one of AOP's existing mechanisms like exclude "org.jboss.ejb3.*"
                                from having aspects applied to it (that can be configured on the AspectManager).

                                That way AOP should be able to "short circuit" the aspect dependency checking
                                for those pojos using the same "filters".

                                The current filters are classname based, which is not a good choice for this. Although jaikiran says otherwise, AFAICR the filters are turned off for proxies anyway, they only apply to weaving classes.

                                What exactly do you mean by "current filters are classname based"?
                                You cannot do globing (org.acme.foobar.*) like Adrian suggests?

                                • 28. Re: DESCRIBE phase - Dependency builders for MC Beans
                                  alesj

                                   

                                  "jason.greene@jboss.com" wrote:

                                  but if the performance sucks as bad as this does, no one will use it.

                                  Didn't you use this with PojoCache, but never noticed the bad performance? ;-)

                                  "jason.greene@jboss.com" wrote:

                                  In general we should avoid expanding our usage of this until we know that the design is even fixable.

                                  If Google can search a googol of entries in a split sec,
                                  I don't see why we couldn't do the same for a handful of beans and aspects.

                                  Kabir, what's the current search/matching mechanism?
                                  Can it be made plugable?
                                  Or how much would it take to improve that?

                                  Idea of Lucene or Hadoop (on a single cpu) comes to my mind ... :-)
                                  Or basically any decent BTrees impl should do.

                                  • 29. Re: DESCRIBE phase - Dependency builders for MC Beans
                                    alesj

                                    I've applied a patch made by Kabir.

                                    There is now an @DisableAOP annotation, for which we check in AOPDB and AOPCJ.
                                    If present, we fall back to super behavior.