1 2 Previous Next 15 Replies Latest reply on Jun 26, 2008 11:00 AM by flavia.rainone

    Optimizing Pointcut Matching

    stalep

      with the fix for JBAOP-433 flavia had some ideas for optimizing the pointcut matching;
      suggestion for optimization is to classify pointcuts.
      - "execution(blahblahblah)" would be classified as Execution
      - "field(...)", classified as field read and field write
      - "call(...) OR execution(..)", classified as call and as execution
      this classification would allow the Advisor to get only the relevant pointcut expressions during an interceptor chain build process.

      a few questions:
      - how fine grained we should classify the pointcuts?
      - could this change improve performance in other areas of aop other than rebuilding the stacks?
      - should we create a special pointcutcollection class (as flavia suggested) that contain the pointcuts and with utilities to fetch them as needed? (basically we should avoid to clutter up AM anymore than it already is)
      - is there any gain with refactoring out everything thats involving pointcuts from AM?

        • 1. Re: Optimizing Pointcut Matching
          kabirkhan

           


          - how fine grained we should classify the pointcuts?

          I think if we are going to do this, we should be able to classify the pointcuts fully. The expression tree from javacc when we parse a Pointcut from the string representation already contains the full structure, so I guess it is just a case of creating another visitor to classify the pointcut when we add the pointcut to the AM. We should split the "execution" classification into whether a method or constructor is called as well.

          - should we create a special pointcutcollection class (as flavia suggested) that contain the pointcuts and with utilities to fetch them as needed? (basically we should avoid to clutter up AM anymore than it already is)

          Sounds like a good idea, although we need to keep the existing API since we have gone CR. The old methods could just wrap the new structures.

          If we add a new binding containing an execution pointcut for methods, then as a result of that the affected advisors should only rebuild their method chains, no point in rebuilding constructors, fields etc.

          On that note, if adding a field read/write pointcut, we should ignore all Advisors that cannot have advised fields, such as the ClassContainer and ClassProxyContainer.



          • 2. Re: Optimizing Pointcut Matching
            kabirkhan

            We discussed something interesting in Orlando that Flavia brought up. When adding a binding, why are we cleaning out ALL the chains for the affected classadvisors and then iterating over bindings for each joinpoint for that class?

            If we are adding a binding, we could just do the matching for that binding and see if it matches anything within the affected advisors, add it to the end of the interceptor chain if it does, and then sort according to precedence. That will be a lot faster since we don't have to match every joinpoint against every pointcut in the system whenever we add/remove something.

            We decided not to do this in Orlando, since the joinpoint graph would handle this. Since the joinpoint graph is not ready yet, I think we should go forward with this solution. We will need to do some additional housekeeping to see which interceptors in the advisor's JoinPointInfo's belong to which AspectBindings, so that we can remove bindings easily without having to recalculate everything.

            I think that only time we should need to recalculate everything is if we add annotation overrides or metadata to an advisor, since then it might match more pointcuts



            • 3. Re: Optimizing Pointcut Matching
              kabirkhan

              I have implemented this for ClassContainers and ClassAdvisors
              http://jira.jboss.com/jira/browse/JBAOP-575

              See the linked JIRA issues for outstanding tasks related to this. For AS startup, I have only focused on ClassContainers for now, since they are used by the proxies used in AS 5, i.e. there is no weaving involved.

              For the "default" config on my machine, the startup time has gone from 34s to 28s. For "all" from 48s to 42s.

              Basically when adding bindings, we only add the new binding to the end of the chains for the existing advisors. This causes slightly different behaviour if we have an advisor with an instance domain in GeneratedAdvisor mode, since then bindings for the class-level domain are handled before the bindings for the instance domain. Now, the last added is added to the end of the chain regardless of what domain level it is added to. However, precedence rules are still calculated following an added binding, so that can be used to determine the order where critical.

              • 4. Re: Optimizing Pointcut Matching
                flavia.rainone

                 

                For the "default" config on my machine, the startup time has gone from 34s to 28s. For "all" from 48s to 42s.

                Great news!

                Do you think it is worthy to implement the joinpoint classification I suggested?

                • 5. Re: Optimizing Pointcut Matching
                  kabirkhan

                  If you have time, then I think this will be a good thing to have.

                  http://jira.jboss.com/jira/browse/JBAOP-579 I have expanded the PointcutStats class, which already did some classification, to classify things more finely. I added the methodExecution/Call and construtorExecution/Call fields. You can see this used in PointcutTestCase.testPointcutClassification().

                  It should now be a matter of plugging it into the advisors so we only match what we need to match.

                  • 6. Re: Optimizing Pointcut Matching
                    flavia.rainone

                     

                    If you have time, then I think this will be a good thing to have.


                    In this case, I'm going to implement it this week. Let's hope it improves the performance somewhat.

                    I have expanded the PointcutStats class, which already did some classification, to classify things more finely.


                    Thanks! That's the most difficult part of it. Plugging it in should be easy now.

                    • 7. Re: Optimizing Pointcut Matching
                      kabirkhan

                      ReflectiveAspectBinder will also need to be updated to only match on what is relevant. It is used heavily by the MC during the "describe" phase to determine the extra dependencies brought in from aspects applied to beans

                      • 8. Re: Optimizing Pointcut Matching
                        jesper.pedersen

                        I've attached a profile of jboss-5.0.0.CR1-rev73546 to http://jira.jboss.com/jira/browse/JBVFS-26 where you can see who calls who.

                        Maybe that is useful for this work also.

                        • 9. Re: Optimizing Pointcut Matching
                          kabirkhan

                           

                          "kabir.khan@jboss.com" wrote:
                          ReflectiveAspectBinder will also need to be updated to only match on what is relevant. It is used heavily by the MC during the "describe" phase to determine the extra dependencies brought in from aspects applied to beans

                          This has been implemented

                          • 10. Re: Optimizing Pointcut Matching
                            flavia.rainone

                             

                            It should now be a matter of plugging it into the advisors so we only match what we need to match.


                            The BindingClassifier is plugged in the class advisor. However, my initial suggestion was a little different from what we have implemented. My suggestion consisted of keeping pointcuts classified into collections, so that it would be a matter of just retrieving the appropriate collection to perform the matching. Here is a piece of an e-mail I sent to the aop-dev list:

                            So, for example, when ClassAdvisor is building a chain for fieal read, it will do something like AspectManager.getFieldReadPointcuts(), or AspectManager.getPointcuts(PointcutType.FIELD_READ). It would match only those pointcuts to build the chains for field read joinpoints.

                            To avoid incrementing the AspectManager interface too much, the methods added to AspectManager should be package protected. Another possibility would be adding a single method to AspectManager:
                            PointcutCollection pointcuts = AspectManager.getPointcutCollection();

                            Whereas PointcutCollection is a package-protected class, and this class would contain the methods to get all the specific pointcuts collection: getExecutionPointcuts, getFieldReadPointcuts, and so on. Notice that, whatever we add to AspectManager, can and should be package-proctected. So, the method above would be package-protected as well.


                            This approach would avoid the extra cost of doing an if statement for each binding to be matched. What do you think?

                            • 11. Re: Optimizing Pointcut Matching
                              kabirkhan

                              If this can be made so that the bindings are applied in the same order as the current "unsorted" way that's fine. That could probably work by for example
                              *all "methodexecution" and "all" bindings are added to the "methodexecution" collextion
                              *all "field", "all" and "set" bindings are added to the "set" collection
                              etc.

                              Although, this might get a bit complicated when using sub-domains? Take a look at Domain.getBindings() and see if you can come up with something there

                              • 12. Re: Optimizing Pointcut Matching
                                flavia.rainone

                                 


                                In this case, I'm going to implement it this week. Let's hope it improves the performance somewhat.

                                This is implemented now. I am going to commit soon.

                                I think that, during implementation, I have spotted a bug. InstanceDomain class, overwites addBinding method. Isn't it supposed to make pointcuts collection consistent, by adding the binding to the pointcuts collection as well?

                                Plus, do you think it would be worthy to use the classification on the instrumentation stages as well? We could have a classified pointcut collection, similar to the one I wrote for bindings, and the transformers would match only the relevant pointcuts.

                                • 13. Re: Optimizing Pointcut Matching
                                  kabirkhan

                                   

                                  "flavia.rainone@jboss.com" wrote:
                                  I think that, during implementation, I have spotted a bug. InstanceDomain class, overwites addBinding method. Isn't it supposed to make pointcuts collection consistent, by adding the binding to the pointcuts collection as well?

                                  Probably

                                  "flavia.rainone@jboss.com" wrote:
                                  Plus, do you think it would be worthy to use the classification on the instrumentation stages as well? We could have a classified pointcut collection, similar to the one I wrote for bindings, and the transformers would match only the relevant pointcuts.

                                  Yes, I think this is worth implementing. I thought of this while away, but forgot by the time I got back :-(

                                  • 14. Re: Optimizing Pointcut Matching
                                    flavia.rainone

                                     

                                    Probably

                                    I'll fix locally and check if no tests get broken.

                                    Yes, I think this is worth implementing. I thought of this while away, but forgot by the time I got back :-(

                                    I have created a task to use the classification during instrumentation.
                                    http://jira.jboss.com/jira/browse/JBAOP-603


                                    1 2 Previous Next