1 2 Previous Next 19 Replies Latest reply on May 5, 2009 10:04 AM by Brian Stansberry

    Caching of classes in BaseClassLoaderDomain

    Brian Stansberry Master

      Apologies if this topic has been discussed/resolved elsewhere. TBH I'm sure it has. If so, please just tell me to go search. But in any case the implications are big enough that it seems worth another discussion.

      Dominik Pospisil is running some perf tests and is seeing a roughly 10x degradation in the # of ejb3 sfsb requests/sec a cluster can handle w/ 5.1.0.Beta1 vs EAP 4.3. A thread dump on one of the nodes shows 300 threads contending for a BaseClassLoader object's monitor. The contention is through the loadClass(String, boolean) method, synchronized(this) statement at line 411.

      I believe the key issue here is we are not caching classes in BaseClassLoaderDomain. Following stack trace from the thread that currently holds the monitor illustrates:

      "WorkerThread#189[10.16.88.183:37775]" prio=10 tid=0x66c81000 nid=0x5645 runnable [0x5c184000..0x5c185eb0]
       java.lang.Thread.State: RUNNABLE
       at java.lang.Throwable.fillInStackTrace(Native Method)
       - locked <0xe74f8600> (a java.security.PrivilegedActionException)
       at java.lang.Throwable.<init>(Throwable.java:241)
       at java.lang.Exception.<init>(Exception.java:77)
       at java.security.PrivilegedActionException.<init>(PrivilegedActionException.java:48)
       at java.security.AccessController.doPrivileged(Native Method)
       at java.net.URLClassLoader.findClass(URLClassLoader.java:188)
       at java.lang.ClassLoader.loadClass(ClassLoader.java:307)
       - locked <0x756a3fe8> (a sun.misc.Launcher$AppClassLoader)
       at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:301)
       - locked <0x756a3fe8> (a sun.misc.Launcher$AppClassLoader)
       at java.lang.ClassLoader.loadClass(ClassLoader.java:300)
       - locked <0x757cc198> (a org.jboss.system.NoAnnotationURLClassLoader)
       at java.lang.ClassLoader.loadClass(ClassLoader.java:252)
       at java.lang.ClassLoader.loadClassInternal(ClassLoader.java:320)
       - locked <0x757cc198> (a org.jboss.system.NoAnnotationURLClassLoader)
       at java.lang.Class.forName0(Native Method)
       at java.lang.Class.forName(Class.java:247)
       at org.jboss.classloader.plugins.loader.ClassLoaderToLoaderAdapter.loadClass(ClassLoaderToLoaderAdapter.java:172)
       at org.jboss.classloader.spi.ClassLoaderDomain.loadClassFromParent(ClassLoaderDomain.java:352)
       at org.jboss.classloader.spi.ClassLoaderDomain.loadClassBefore(ClassLoaderDomain.java:307)
       at org.jboss.classloader.spi.base.BaseClassLoaderDomain.loadClass(BaseClassLoaderDomain.java:246)
       at org.jboss.classloader.spi.base.BaseClassLoaderDomain.loadClass(BaseClassLoaderDomain.java:1102)
       at org.jboss.classloader.spi.base.BaseClassLoader.loadClassFromDomain(BaseClassLoader.java:772)
       at org.jboss.classloader.spi.base.BaseClassLoader.loadClass(BaseClassLoader.java:415)
       - locked <0x79d9a068> (a org.jboss.classloader.spi.base.BaseClassLoader)
       at java.lang.ClassLoader.loadClass(ClassLoader.java:252)
       at org.jboss.ejb3.metadata.annotation.AnnotationRepositoryToMetaData.loadClass(AnnotationRepositoryToMetaData.java:216)
       at org.jboss.ejb3.metadata.annotation.AnnotationRepositoryToMetaData.hasAnnotation(AnnotationRepositoryToMetaData.java:328)
       at org.jboss.aop.Advisor.hasAnnotation(Advisor.java:889)
       at org.jboss.aop.Advisor.hasAnnotation(Advisor.java:865)
       at org.jboss.ejb3.interceptors.aop.LifecycleCallbacks.createLifecycleCallbackInterceptors(LifecycleCallbacks.java:101)
       at org.jboss.ejb3.EJBContainer.invokeCallback(EJBContainer.java:1112)
       at org.jboss.ejb3.stateful.StatefulContainer.invokePrePassivate(StatefulContainer.java:668)
       at org.jboss.ejb3.stateful.StatefulBeanContext.preReplicate(StatefulBeanContext.java:544)
       at org.jboss.ejb3.cache.tree.StatefulTreeCache.putInCache(StatefulTreeCache.java:510)
       at org.jboss.ejb3.cache.tree.StatefulTreeCache.create(StatefulTreeCache.java:123)
       at org.jboss.ejb3.stateful.StatefulContainer.createSession(StatefulContainer.java:388)
       at org.jboss.ejb3.session.SessionContainer.createSession(SessionContainer.java:677)
       at org.jboss.ejb3.proxy.factory.session.stateful.StatefulSessionProxyFactoryBase.getNewSessionId(StatefulSessionProxyFactoryBase.java:296)
       at org.jboss.ejb3.proxy.factory.session.stateful.StatefulSessionProxyFactoryBase.createProxyBusiness(StatefulSessionProxyFactoryBase.java:160)
       at sun.reflect.GeneratedMethodAccessor304.invoke(Unknown Source)
       at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
       at java.lang.reflect.Method.invoke(Method.java:597)
       at org.jboss.aop.Dispatcher.invoke(Dispatcher.java:121)
       at org.jboss.aspects.remoting.AOPRemotingInvocationHandler.invoke(AOPRemotingInvocationHandler.java:82)
       at org.jboss.remoting.ServerInvoker.invoke(ServerInvoker.java:908)
       at org.jboss.remoting.transport.socket.ServerThread.completeInvocation(ServerThread.java:742)
       - locked <0x805023c8> (a org.jboss.remoting.transport.socket.ServerThread)
       at org.jboss.remoting.transport.socket.ServerThread.processInvocation(ServerThread.java:695)
       at org.jboss.remoting.transport.socket.ServerThread.dorun(ServerThread.java:549)
       at org.jboss.remoting.transport.socket.ServerThread.run(ServerThread.java:230)


      Here we have an EJB3 SFSB deployment, no loader repository configured so it is using the default classloading domain. The call stack is trying to load class javax.ejb.PrePassivate, which wasn't initialized by the EJB3 deployment's classloader, so the deployment's BaseClassLoader is asking the BaseClassLoadingDomain for the class. The class has already been loaded, by whatever classloader loaded common/lib/jboss-javaee.jar.

      This is an extremely common call pattern. AFAICT to return the already loaded class in EAP 4.3 vs 5.x we have, in simplified form:

      EAP 4.3

      a single get from a concurrent hash map:

      return UnifiedLoaderRepository3.classes.get(name);

      AS 5.x

      1) Ask the parent for the class, which calls ClassLoader.loadClass() proceeding all the way to the bootstrap classloader, executing privileged blocks throwing CNFEs wrapped inside PrivilegedActionExceptions at each level and then catching and discarding them. The above stack trace catches a thread in the middle of creating a PrivilegedActionException.
      2) Executing BaseClassLoaderDomain.findLoader logic to find the Loader that loaded the class.
      3) Scheduling and processing ClassLoadingTask to get the class from the Loader.

      Yikes! If the 4.x approach is too much of a "big ball of mud" can't we at least find a way to skip step #1 if the class was already loaded from one of the loaders associated with the domain?

      In forum thread at http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4218813#4218813 pbmwg reports a similar problem, with a 66% performance improvement via eliminating just one source of calls to BaseClassLoader.loadClass().

        • 1. Re: Caching of classes in BaseClassLoaderDomain
          Adrian Brock Master

           

          "bstansberry@jboss.com" wrote:

          EAP 4.3

          a single get from a concurrent hash map:

          return UnifiedLoaderRepository3.classes.get(name);

          AS 5.x

          1) Ask the parent for the class, which calls ClassLoader.loadClass() proceeding all the way to the bootstrap classloader, executing privileged blocks throwing CNFEs wrapped inside PrivilegedActionExceptions at each level and then catching and discarding them. The above stack trace catches a thread in the middle of creating a PrivilegedActionException.
          2) Executing BaseClassLoaderDomain.findLoader logic to find the Loader that loaded the class.
          3) Scheduling and processing ClassLoadingTask to get the class from the Loader.

          Yikes! If the 4.x approach is too much of a "big ball of mud" can't we at least find a way to skip step #1 if the class was already loaded from one of the loaders associated with the domain?


          Yes, I agree. There already is a "globalClassCache" when it emulates the
          "big ball of mud", but the processing of this is inefficient as you mention
          i.e. it caches the Loader not the Class and goes through the task scheduling inside the
          classloader lock.

          I'll change the checking of the cache to be more like JBoss4.x
          although the actual population of the cache needs to remain similar to what it now
          since even the "big ball of mud" can be tweeked such as turning off caching
          for individual classloaders or choosing not to export everything.
          https://jira.jboss.org/jira/browse/JBCL-100

          • 2. Re: Caching of classes in BaseClassLoaderDomain
            Adrian Brock Master

            I've uploaded a snapshot with this fix.
            All you really need to do is jboss-classloader.jar with the one from here:
            http://snapshots.jboss.org/maven2/org/jboss/cl/jboss-classloader/2.0.6-SNAPSHOT/
            to try it out.

            • 3. Re: Caching of classes in BaseClassLoaderDomain
              Brian Stansberry Master

              Sorry for slow reply; I got e-mail messages for other postings on this "Design of POJO Server" forum yesterday, but none for this topic. :(

              Thanks for the quick patch; will get it tested out.

              • 4. Re: Caching of classes in BaseClassLoaderDomain
                Brian Stansberry Master

                Dominik is running the EJB3 perf tests and while it's early, he reports that "but the app is running, and it's running much much faster :)"

                The "but" above is because there are CNFE problems at server start; look to be due to webapps being unable to load classes from the domain. Key server.log snippet:

                2009-04-28 10:43:38,715 DEBUG [org.jboss.web.tomcat.service.deployers.TomcatDeployment] (main) injectionContainer enabled and processing beginning
                2009-04-28 10:43:38,716 ERROR [org.apache.catalina.core.ContainerBase.[jboss.web].[localhost].[/admin-console]] (main) Error configuring application listener of class org.jboss.web.tomcat.security.SecurityFlushSessionListener
                java.lang.ClassNotFoundException: org.jboss.web.tomcat.security.SecurityFlushSessionListener
                 at java.net.URLClassLoader$1.run(URLClassLoader.java:200)
                 at java.security.AccessController.doPrivileged(Native Method)
                 at java.net.URLClassLoader.findClass(URLClassLoader.java:188)
                 at java.lang.ClassLoader.loadClass(ClassLoader.java:306)
                 at java.lang.ClassLoader.loadClass(ClassLoader.java:251)
                 at org.jboss.web.tomcat.service.TomcatInjectionContainer.newInstance(TomcatInjectionContainer.java:262)
                 at org.jboss.web.tomcat.service.TomcatInjectionContainer.newInstance(TomcatInjectionContainer.java:256)
                 at org.apache.catalina.core.StandardContext.listenerStart(StandardContext.java:3859)
                 at org.apache.catalina.core.StandardContext.start(StandardContext.java:4393)
                 at org.jboss.web.tomcat.service.deployers.TomcatDeployment.performDeployInternal(TomcatDeployment.java:310)
                 at org.jboss.web.tomcat.service.deployers.TomcatDeployment.performDeploy(TomcatDeployment.java:142)
                 at org.jboss.web.deployers.AbstractWarDeployment.start(AbstractWarDeployment.java:461)
                 at org.jboss.web.deployers.WebModule.startModule(WebModule.java:118)
                 at org.jboss.web.deployers.WebModule.start(WebModule.java:97)


                The SecurityFlushSessionListener class is located in server/all/deploy/jboss-web.sar/jboss-web-service.jar.

                Above is immediately followed by same thing 3 more times for other classes:

                java.lang.ClassNotFoundException: org.jboss.web.jsf.integration.config.JBossJSFConfigureListener
                java.lang.ClassNotFoundException: com.sun.faces.application.WebappLifecycleListener
                java.lang.ClassNotFoundException: com.sun.faces.config.ConfigureListener

                Same thing happens on ROOT.war and jmx-console.war.

                • 5. Re: Caching of classes in BaseClassLoaderDomain
                  Brian Stansberry Master

                  The webapps that fail are declaring a loader-repository in jboss-web.xml. The CNFE is coming from:

                  Thread [main] (Suspended)
                   ClassLoaderDomain(BaseClassLoaderDomain).checkClassBlackList(BaseClassLoader, String, String, boolean) line: 1477
                   ClassLoaderDomain(BaseClassLoaderDomain).checkClassCacheAndBlackList(BaseClassLoader, String, String, boolean) line: 1503
                   BaseClassLoader.checkCacheAndBlackList(String, boolean) line: 365
                   BaseClassLoader.loadClass(String, boolean) line: 429
                   WebCtxLoader$ENCLoader(ClassLoader).loadClass(String, boolean) line: 299
                   WebCtxLoader$ENCLoader(ClassLoader).loadClass(String) line: 251
                   TomcatInjectionContainer.newInstance(String, ClassLoader) line: 262


                  The requested classes are in the domain's black list.

                  I'm assuming this CNFE should be caught and the call allowed to continue on to check the parent domain. I'll look at the JBCL-100 patch to see where the likely place to catch it is.

                  • 6. Re: Caching of classes in BaseClassLoaderDomain
                    Brian Stansberry Master

                    Just to experiment I wrapped the call to BaseClassLoader.checkCacheAndBlackList in a try/catch and caught the CNFE. The AS starts clean and the smoke tests pass. A mvn install in the jboss-cl codebase shows 3 tests with errors, but those were there before I made the change as well.

                    Looking at the actual usage, the catch could also be inside checkCacheAndBlackList itself, but that's a protected method that declares the exception in its signature.

                    I'm not committing this as you may have other thoughts and if not it's trivial.

                    Now running classloader leak tests...

                    • 7. Re: Caching of classes in BaseClassLoaderDomain
                      Brian Stansberry Master

                      The classloader leak tests pass.

                      • 8. Re: Caching of classes in BaseClassLoaderDomain
                        Dominik Pospisil Newbie

                        Results of the perf test with provided fix are available at:
                        http://hudson.qa.jboss.com/hudson/view/EAP Perf+Fail/job/as5-perf-ejb3-buddy-async/51/artifact/report/report.html

                        without fix:
                        http://hudson.qa.jboss.com/hudson/view/EAP Perf+Fail/job/as5-perf-ejb3-buddy-async/48/artifact/report/report.html

                        So the test is running 5-10x faster. Still slower, but now the results are comparable to EAP 4.3

                        • 9. Re: Caching of classes in BaseClassLoaderDomain
                          Brian Stansberry Master

                          Comparable, but TPS is still only 63% of what I see for EAP 4.3 at http://hudson.qa.jboss.com/hudson/view/EAP%20Perf+Fail/job/eap-4.3-perf-ejb3-buddy-async/lastSuccessfulBuild/artifact/report/report.html. So we still have a long way to go. But the slowdown could be for many reasons.

                          • 11. Re: Caching of classes in BaseClassLoaderDomain
                            Ales Justin Master

                            I don't think you should solve this via catching the CNFE.
                            The fix should be as you stated in JIRA - additional parameter.
                            This is new code anyway, so adding extra parameters should not present any problems.

                            • 12. Re: Caching of classes in BaseClassLoaderDomain
                              Brian Stansberry Master

                              Done. That would be this:

                              Index: BaseClassLoader.java
                              ===================================================================
                              --- BaseClassLoader.java (revision 88165)
                              +++ BaseClassLoader.java (working copy)
                              @@ -351,18 +351,22 @@
                               * Check the cache and blacklist
                               *
                               * @param name the name of the class
                              + * @param failIfBlackListed <code>true</code> if a blacklisted class should
                              + * result in ClassNotFoundException; <code>false</code>
                              + * if a <code>null</code> return value is acceptable
                               * @param trace whether trace is enabled
                               * @return the class is if it is already loaded, null otherwise
                              - * @throws ClassNotFoundException when blacklisted
                              + * @throws ClassNotFoundException when the class is blacklisted and
                              + * <code>failIfBlackListed</code> is <code>true</code>
                               */
                              - protected Class<?> checkCacheAndBlackList(String name, boolean trace) throws ClassNotFoundException
                              + protected Class<?> checkCacheAndBlackList(String name, boolean failIfBlackListed, boolean trace) throws ClassNotFoundException
                               {
                               BaseClassLoaderPolicy basePolicy = policy;
                               BaseClassLoaderDomain domain = basePolicy.getClassLoaderDomain();
                               if (domain == null)
                               return null;
                              
                              - return domain.checkClassCacheAndBlackList(this, name, null, basePolicy.isImportAll());
                              + return domain.checkClassCacheAndBlackList(this, name, null, basePolicy.isImportAll(), false);
                               }
                              
                               /**
                              @@ -426,17 +430,9 @@
                               if (result != null)
                               return result;
                              
                              - try
                              - {
                              - result = checkCacheAndBlackList(name, trace);
                              - if (result != null)
                              - return result;
                              - }
                              - catch (ClassNotFoundException blacklisted)
                              - {
                              - if (trace)
                              - log.trace(name + " has been blacklisted; cannot load from domain cache");
                              - }
                              + result = checkCacheAndBlackList(name, false, trace);
                              + if (result != null)
                              + return result;
                              
                               synchronized (this)
                               {
                              Index: BaseClassLoaderDomain.java
                              ===================================================================
                              --- BaseClassLoaderDomain.java (revision 88158)
                              +++ BaseClassLoaderDomain.java (working copy)
                              @@ -1487,10 +1487,14 @@
                               * @param name the name
                               * @param path the path of the class resource
                               * @param allExports whether to look at all exports
                              + * @param failIfBlackListed <code>true</code> if a blacklisted class should
                              + * result in ClassNotFoundException; <code>false</code>
                              + * if a <code>null</code> return value is acceptable
                               * @return the class when found in the cache
                              - * @throws ClassNotFoundException when the class is blacklisted
                              + * @throws ClassNotFoundException when the class is blacklisted and
                              + * <code>failIfBlackListed</code> is <code>true</code>
                               */
                              - protected Class<?> checkClassCacheAndBlackList(BaseClassLoader classLoader, String name, String path, boolean allExports) throws ClassNotFoundException
                              + protected Class<?> checkClassCacheAndBlackList(BaseClassLoader classLoader, String name, String path, boolean allExports, boolean failIfBlackListed) throws ClassNotFoundException
                               {
                               if (path == null)
                               path = ClassLoaderUtils.classNameToPath(name);
                              @@ -1498,8 +1502,11 @@
                               Class<?> result = checkClassCache(classLoader, name, path, allExports);
                               if (result != null)
                               return result;
                              -
                              - checkClassBlackList(classLoader, name, path, allExports);
                              +
                              + if (failIfBlackListed)
                              + {
                              + checkClassBlackList(classLoader, name, path, allExports);
                              + }
                               return null;
                               }
                              


                              Which is simple enough. I'm not sure if there's a use case for failIfBlackListed=true, but it's exposed if there is one.


                              • 13. Re: Caching of classes in BaseClassLoaderDomain
                                Ales Justin Master

                                I moved the failIfBlackListed flag chek into the checkClassBlackList method,
                                as I think it should be its responsibility to determine what to do with it.
                                (e.g. some other domain impl might handle this diff ... although not very likely :-))

                                + void checkClassBlackList(BaseClassLoader classLoader, String name, String path, boolean allExports, boolean failIfBlackListed) throws ClassNotFoundException
                                 {
                                 if (allExports)
                                 {
                                - if (globalClassBlackList.containsKey(path))
                                + if (failIfBlackListed && globalClassBlackList.containsKey(path))
                                 {
                                 if (log.isTraceEnabled())
                                 log.trace("Found " + name + " in global blacklist");
                                @@ -1491,7 +1494,7 @@
                                 * result in ClassNotFoundException; <code>false</code>
                                 * if a <code>null</code> return value is acceptable
                                 * @return the class when found in the cache
                                - * @throws ClassNotFoundException when the class is blacklisted and
                                + * @throws ClassNotFoundException when the class is blacklisted and
                                 * <code>failIfBlackListed</code> is <code>true</code>
                                 */
                                 protected Class<?> checkClassCacheAndBlackList(BaseClassLoader classLoader, String name, String path, boolean allExports, boolean failIfBlackListed) throws ClassNotFoundException
                                @@ -1502,11 +1505,9 @@
                                 Class<?> result = checkClassCache(classLoader, name, path, allExports);
                                 if (result != null)
                                 return result;
                                -
                                - if (failIfBlackListed)
                                - {
                                - checkClassBlackList(classLoader, name, path, allExports);
                                - }
                                +
                                + checkClassBlackList(classLoader, name, path, allExports, failIfBlackListed);
                                +


                                • 14. Re: Caching of classes in BaseClassLoaderDomain
                                  Adrian Brock Master

                                  Well, turning the blacklist off altogether is one way to fix it. :-)

                                  But it is inconsistent. If you had three levels of hierarchical domain it still
                                  wouldn't work because loadClass(...) still checks the blacklist.

                                  The issue is that the blacklist is being checked before the parent (cache) is asked
                                  wether it knows the class.

                                  The order should be more like:

                                  1) Check the current domain to see if cached == false
                                  2) Check the parent domain to see if cached == true
                                  3) Check the current domain to see if blacklisted == true

                                  Currently (2) and (3) are the wrong way around. Your fix just removes (3)

                                  In fact, the real issue is that (2) is not happening at all except inside the classloader
                                  lock which is not very efficient.

                                  I'll fix it properly, but it is slightly complicated in that step (2) isn't the
                                  correct thing to do if the class doesn't match the parent filter.

                                  1 2 Previous Next