Community
Participate
Working Groups
The attached patch allows reusing weaving adaptors for sun.reflect.DelegatingClassLoader loaders, which noticably improves load-time weaving performance and is always safe. I found that in a typical app server environment, there can be a lot of sun.reflect.DelegatingClassLoader instances lying around. They appear to be generated by the Sun reflection implementation and are used to load a single generated acessor or method. http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4474172 has some information on what these loaders do. The key thing is that there will never be an aop.xml file in one of them, so it's safe to share for them... In my memory optimized load-time weaving set up not creating separate worlds reduced heap overhead by a lot, e.g., 200%, since most of the overhead in my optimized version comes from loading worlds. This allowed me to go from 14 world instances to just 3. In the version in CVS head, it allowed me to reduce overhead on another project from 90 mb to 70 mb and to reduce start-up times by 5-10%.
Created attachment 31599 [details] use the parent loader's adaptor, or skip weaving if null (nothing is woven into reflection delegates from the bootstrap loader)
Created attachment 31601 [details] Cleaner version that just skips trying to weave in reflection loaders altogether. The earlier patch works because we will never be weaving into the reflection loader loaded types. This version makes that clearer & is a little faster.
An alternative strategy that also works is to make WeavingAdaptor.shouldWeaveName a public and static, and then have Aj.preProcess start with: if (loader == null || className == null || !WeavingAdaptor.shouldWeaveName(className.replace('/','.'))) { This imposes a bit more overhead (requiring a check on each class being loaded) but it removes the dependency on the implementation class sun.reflect.DelegatingClassLoader
Which of the following are you suggesting? 1. Don't create a Weaver for each DelegatingClassLoader 2. Weaving reflection delegates is unlikely/impossible so optimize for DelegatingClassLoaders by sharing state 3. Don't permit weaving of reflection delegates Your patch seems to implement 3 while the summary requests 1 and the description describes 2. I agree with the solution in your 2nd patch rather than your comment concerning a new public method. Reflection delegates, unlike user generated Java proxies, can be considered part of the JVM implementation and should not be exposed to the weaver. A safe and efficient test in Aj.preProcess() would be the best approach.
Ron, I am not sure that this patch will give you the benefit that you suggest. I have written a testcase the causes the creation of a DelegatingClassLoader, an accompanying WeavingAdaptor and generated sun/reflect/XXX classes. However with fixes to bug 113511, bug 116626 and bug 117189 (all of which use the "enabled" flag) we find no aop.xml, issue a message and return without creating a World or Weaver. All subsequent calls to "WeavingAdaptor.weaveClass()" will return immediately. Have you picked up changes from CVS head recently?
Yes I have tested this with the latest version. The performance gains are noticable. There are cases where you make reflective calls from within a ClassLoader where an aop.xml is visible (e.g., through a parent), i.e., the DelegatingClassLoader has a parent. I will try to provide a simple example ... if you can provide me a patch that reproduces your tests that would be helpful. To the earlier question, I realized that there's no point in creating a weaver to weave reflection delegates because WeaverAdaptor.shouldWeaveName never does so. The first patch implemented #1 to achieve #2: it used the parent loader's adaptor (this actually might break if we generated types like around closures in the delegating loader).
Ron, I am still puzzled. According to the description of a DelegationClassLoader "The key thing is that there will never be an aop.xml file in one of them ...", which is confirmed by my test, any attached WeavingAdaptor will be disabled. Your patch will simply prevent the creation of the adaptor. Are you saying that there are circumstances when a DelegatingClassLoader _can_ see an aop.xml or are you saying the footprint of the adaptor itself is high?
Here's an example I'm seeing inside of Tomcat. Basically, the DelegatingClassLoader has a parent and it delegates to the parent to discover the various aop.xml files. While there is not (and never can be) an aop.xml file inside the DelegatingClassLoader, it will use normal classloader delegation to find one from its parent loader. As a result, the system pays a fairly hefty price of parsing and constructing a big tree of BCEL objects for every such delegating class loader. Just on startup, I found a single Web app had 75% of the total enabled worlds coming from these reflection delegate class loaders. loader.parent is an org.apache.catalina.loader.WebappClassLoader definitions includes all of the aspects I've deployed to shared/lib The first stack trace where I see a non-empty set of definitions is: ClassLoaderWeavingAdaptor.initialize(ClassLoader, IWeavingContext) line: 107 Aj$ExplicitlyInitializedClassLoaderWeavingAdaptor.initialize(ClassLoader, IWeavingContext) line: 135 Aj$ExplicitlyInitializedClassLoaderWeavingAdaptor.getWeavingAdaptor(ClassLoader, IWeavingContext) line: 140 Aj$WeaverContainer.getWeaver(ClassLoader, IWeavingContext) line: 106 Aj.preProcess(String, byte[], ClassLoader) line: 62 ClassPreProcessorAgentAdapter.transform(ClassLoader, String, Class<?>, ProtectionDomain, byte[]) line: 52 TransformerManager.transform(ClassLoader, String, Class, ProtectionDomain, byte[]) line: 122 InstrumentationImpl.transform(ClassLoader, String, Class, ProtectionDomain, byte[]) line: 155 Unsafe.defineClass(String, byte[], int, int, ClassLoader, ProtectionDomain) line: not available [native method] ClassDefiner.defineClass(String, byte[], int, int, ClassLoader) line: 45 MethodAccessorGenerator$1.run() line: 381 AccessController.doPrivileged(PrivilegedAction<T>) line: not available [native method] MethodAccessorGenerator.generate(Class, String, Class[], Class, Class[], int, boolean, boolean, Class) line: 377 MethodAccessorGenerator.generateConstructor(Class, Class[], Class[], int) line: 76 NativeConstructorAccessorImpl.newInstance(Object[]) line: 30 DelegatingConstructorAccessorImpl.newInstance(Object[]) line: 27 Constructor<T>.newInstance(Object...) line: 494 Class<T>.newInstance0() line: 350 Class<T>.newInstance() line: 303 ObjectCreateRule.begin(AttributeList) line: 153 Digester.startElement(String, AttributeList) line: 528 SAXParser(AbstractSAXParser).startElement(QName, XMLAttributes, Augmentations) line: not available SAXParser(AbstractXMLDocumentParser).emptyElement(QName, XMLAttributes, Augmentations) line: not available XMLDTDValidator.emptyElement(QName, XMLAttributes, Augmentations) line: not available XMLDocumentScannerImpl(XMLDocumentFragmentScannerImpl).scanStartElement() line: not available XMLDocumentScannerImpl$ContentDispatcher(XMLDocumentFragmentScannerImpl$FragmentContentDispatcher).dispatch(boolean) line: not available XMLDocumentScannerImpl(XMLDocumentFragmentScannerImpl).scanDocument(boolean) line: not available XML11Configuration.parse(boolean) line: not available XML11Configuration.parse(XMLInputSource) line: not available SAXParser(XMLParser).parse(XMLInputSource) line: not available SAXParser(AbstractSAXParser).parse(InputSource) line: not available SAXParserImpl(SAXParser).parse(InputSource, HandlerBase) line: 344 SAXParserImpl(SAXParser).parse(InputStream, HandlerBase) line: 120 Digester.parse(InputStream) line: 755 ActionServlet.initMapping() line: 1332 ActionServlet.init() line: 466 ActionServlet(GenericServlet).init(ServletConfig) line: 211 StandardWrapper.loadServlet() line: 1091 StandardWrapper.load() line: 925 StandardContext.loadOnStartup(Container[]) line: 3857 StandardContext.start() line: 4118 StandardHost(ContainerBase).addChildInternal(Container) line: 759 StandardHost(ContainerBase).addChild(Container) line: 739 StandardHost.addChild(Container) line: 524 HostConfig.deployDescriptor(String, File, String) line: 589 HostConfig.deployDescriptors(File, String[]) line: 536 HostConfig.deployApps() line: 471 HostConfig.start() line: 1102 HostConfig.lifecycleEvent(LifecycleEvent) line: 311 LifecycleSupport.fireLifecycleEvent(String, Object) line: 119 StandardHost(ContainerBase).start() line: 1020 StandardHost.start() line: 718 StandardEngine(ContainerBase).start() line: 1012 StandardEngine.start() line: 442 StandardService.start() line: 450 StandardServer.start() line: 683 Catalina.start() line: 537 NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method] NativeMethodAccessorImpl.invoke(Object, Object[]) line: 39 DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 25 Method.invoke(Object, Object...) line: 585 Bootstrap.start() line: 271 Bootstrap.main(String[]) line: 409
patch applied ... thanks.
Ron, I was wrong. You are right. I did not see an aop.xml configuaration for the DelegatingClassLoader (and a World) in my testcase because of the way we configure LTW in the harness when we fork using Ant (which is necessary to use -javaagent). We do not load aop.xml from META-INF but from a location specified using "-Daj.def". However this mechanism only applies to the System class loader (check in ClassLoaderWeavingAdaptor) so the DelegatingClassLoader did not see it. I have revised the testcase to copy the aop.xml to "${aj.sandbox}/META-INF" and now I see the problem. When complete I will attach the testcase.
fix available
