Community
Participate
Working Groups
I propose adding an attribute noMungers="[false]|true" to the aspect element in the aop.xml definition file, e.g., ... <aspects> <aspect name="AbstractNoItd" noMungers="true"/> <aspect name="AbstractWithItd"/> <!-- defaults to false --> <aspect name="Concrete"/> </aspects> This would support an LTW optimization to not load these aspects during register definitions (unless needed for concrete xml-defined aspects). This attribute would be generated when the aop.xml file is generated. More careful testing of the overhead of loading aspects without concrete mungers is needed to assess the performance benefit from this proposal. If it reduces overhead by 10% as my initial quick tests showed, then it's worth considering but after addressing keeping reference types for woven classes and duplicate copies of reference types from common aspect definitions.
Putting user configurable options into files like this is a sign that the programmers are lazy. Why do we have to do it? Why can't we *quickly* determine when first seeing it whether it has 'mungers' in it and do the same thing we are implying with that flag being set to false.
This isn't really intended to be user-configurable, it's intended to be generated. The proposed optimization would avoid the time required to load and parse the file to determine the fact that no concrete mungers are defined.
This enhancement is a bad idea for 2 reasons: 1. Although the flag may be generated it appears in a public configuration file which _could_ be modified by a user with unexpected results: change this flag to make your code go faster (but miss out some of the weaving). 2. Information that is required for a performance optimization (that can be obtained elsewhere i.e. the .class file) should not clutter a user configuration file. However we should discuss the performance issues rather than a particular solution. Since the fix for Bug 120739 we disable the weaver if we can’t find any aspect declarations. Are you concerned that classloaders that can only see abstract aspects will not have their weaver disabled? If so there are a couple of solutions we could implement to ensure that the weaver was disabled if no active aspects were found i.e. no concrete mungers. If however you are concerned about the overhead of loading additional non-active abstract aspects for a classloader that has an enabled weaver then the measurements discussed in Bug 147678 are not very representative. I have measured the time taken to initialize a weaver for a single concrete aspect (8K) and the same aspect along with 4 non-active abstract ones (44K). The times were 631ms and 671ms respectively. This doesn’t seem excessive and could probably be improved without the need for any new flags. I haven’t looked at footprint yet.
That's not so far from 10%. And of course it depends a lot on what the abstract aspects refer to (e.g., how many additional class files need to be loaded and parsed through BCEL). I agree that making something like this user visible isn't ideal, then again the whole aop.xml file and parts like include/exclude are vulnerable to the exact same objection. But let's table this one and focus on higher priority enhancements: if we can load and parse shared aspects once instead of many times (once per loader) then this won't be nearly as important, and that would be far more beneficial.
I don't want to save this one for later as I don't intend to implement the enhancement. More importantly I _still_ don't really know what the problem is (this is the second ehancement request for what is possible a bug). Please answer my questions.
I've answered both questions before: yes and yes. I think 5-10% overhead is a worthy optimization target, but after bigger targets are addressed.
LATER/REMIND bugs are being automatically reopened as P5 because the LATER and REMIND resolutions are deprecated.
