5 Replies Latest reply on May 14, 2010 10:40 AM by alesj

    Classloading and caching changes

    alesj

      It all started as a performance issue.

      A web app is using xerces, trying to load its DocumentBuilder class a lot (really a lot).

      Changing "useLoadClassForParent" on default domain doesn't help a lot.
      And the class isn't cached in domain's globalClassCache.

       

      The problem is that that DB xerces class is loaded in this block (xerces is in JBOSS_HOME/lib), but it's never cached.
      Hence BaseClassLoader always goes into domain and its synch block.

      Couldn't we cache something more inside this block?

       

      Went over the forums these past days, to find anything on the topic, and I found this:
      * http://community.jboss.org/thread/127814?start=15&tstart=0

      When you have one domain delegating to another domain, it doesn't check
      the parent domain cache until after it has entered the synchronization block.

       

      There are two possible fixes:

       

      1) Introduce some extra logic into the domain cache checking that knows
      about the parent delegation. That needs to be done carefully otherwise
      it will check the wrong cache for the parent delegation rules.
      i.e. it needs to differentiate classes loaded from the parent
      before/after the class load in the domain.

       

      2) Cache the result from the parent in the child domain.
      This has the issues mentioned on the forum thread

       

      i) it doesn't differentiate what is loaded before or after the load from
      the target domain
      ii) it complicates the flushing in the event that the parent domain gets
      flushed, e.g. a classloader is removed from the parent domain

       

      I think (1) would be the best way to optimize it, by adding
      checkParentCacheBefore/After() logic into the current cache checking
      that gets invoked outside the synchronization block.

        • 1. Re: Classloading and caching changes
          alesj

          First a few questions.

          What's the idea behind "useLoadClassForParent" flag?
          * So, instead of going into that magic synch code, we try the parent before?

          The loadClassForParent is a backwards compatibility thing to work around
          a few issues that the more efficient resource check causes in some edge cases when it delegates to the java classpath.

           

          It shouldn't be using it when a Domain delegates to another Domain.

           

          That was something we spotted and fixed in 2.2.x, see around line old/new 689/920
          http://fisheye.jboss.org/browse/JBossAS/projects/jboss-cl/trunk/classloader/src/main/java/org/jboss/classloader/spi/ClassLoaderDomain.java?r1=95108&r2=96496

          Why doesn't this code write into global cache?
          * I guess only certain Loaders are valid for caching? e.g. depending on the policy or "OSGi" rules

          (from BaseClassLoaderDomain)

          Loader loader = findLoader(classLoader, path, allExports, findInParent);
               if (loader != null)
               {
                  Thread thread = Thread.currentThread();
                  ClassLoadingTask task = new ClassLoadingTask(name, classLoader, thread);
                  ClassLoaderManager.scheduleTask(task, loader, false);
                  Class<?> result = ClassLoaderManager.process(thread, task);
                  ClassCacheItem item = globalClassCache.get(path);
                  if (item != null)
                     item.clazz = result;
                  return result;
               }

          Whether it caches things is decided elsewhere, based on the ClassLoaderPolicy rules.
          This code is just a safety check in the event that the class changes
          (somebody redeployed a classloader) and the global cache hasn't been flushed properly.

           

          As I said on the other response, it can't cache the load in that "globalClassCache", because
          the classloader used is in the parent domain. It would need some complicated
          flushing rules to cache it in the child domain as well - not my preferred solution.

          • 2. Re: Classloading and caching changes
            alesj
            i) it doesn't differentiate what is loaded before or after the load from

            the target domain
            ii) it complicates the flushing in the event that the parent domain gets
            flushed, e.g. a classloader is removed from the parent domain

            Hmmm ... this code needs fixing then. ;-)

             

                 // Should we directly load from the parent?
                 if (findInParent == false)
                 {
                    Class<?> clazz = loadClassBefore(name);
                    if (clazz != null)
                    {
                       globalClassCache.put(path, new ClassCacheItem(clazz)); // REMOVE
                       return clazz;
                    }
                 }

             

            We as a child don't know when a parent domain has a classloader removed
            and therefore the cached result is invalid.

            It could do if the parent domain flushed all the caches in its child domains, but it doesn't do that currently.
            But even then the globalClassCache wouldn't differentiate the before/after rules.

            • 3. Re: Classloading and caching changes
              alesj
              Whether it caches things is decided elsewhere, based on the ClassLoaderPolicy rules.

              This code is just a safety check in the event that the class changes
              (somebody redeployed a classloader) and the global cache hasn't been flushed properly.

               

              As I said on the other response, it can't cache the load in that "globalClassCache", because
              the classloader used is in the parent domain. It would need some complicated
              flushing rules to cache it in the child domain as well - not my preferred solution.

              "the classloader used is in the parent domain"

               

              But this isn't always the case?
              Only for this piece:

               

                    // Try the before attempt (e.g. from the parent)
                    Loader loader = null;
                    if (findInParent)
                       loader = findBeforeLoader(name);
                    if (loader != null)
                       return loader;

               

              But this could be cached -- if the policy permits it?

               

                    // Next use any requesting classloader, this will look at everything not just what it exports
                    if (classLoader != null)
                    {
                       if (trace)
                          log.trace(this + " trying to load " + name + " from requesting " + classLoader);
                       if (classLoader.getResourceLocally(name) != null)
                          return classLoader.getLoader();
                    }
              • 4. Re: Classloading and caching changes
                alesj
                "the classloader used is in the parent domain"

                 

                But this isn't always the case?
                Only for this piece:

                 

                      // Try the before attempt (e.g. from the parent)
                      Loader loader = null;
                      if (findInParent)
                         loader = findBeforeLoader(name);
                      if (loader != null)
                         return loader;
                

                 

                But this could be cached -- if the policy permits it?

                 

                      // Next use any requesting classloader, this will look at everything not just what it exports
                      if (classLoader != null)
                      {
                         if (trace)
                            log.trace(this + " trying to load " + name + " from requesting " + classLoader);
                         if (classLoader.getResourceLocally(name) != null)
                            return classLoader.getLoader();
                      }
                

                That's why it can't update the Cache in the place you suggested.
                For a classloader inside the domain it can globally cache, otherwise it can't.

                 

                It should cache in the domain where the classloader lives.
                What needs changing is to check that cache according to the parent delegation
                rules outside the synchronization block.

                 

                if (matchesParentBefore)
                  checkParentCache();
                checkOurCache();

                 

                The difficulty comes in that we would like to add
                if (matchesParentAfter)
                  checkParentCache();

                 

                But we can't because the next logical step is to see if we can load from our domain.
                This however would work:

                 

                if (isBlackListedInOurDomain && matchesParentAfter)
                  checkParentCache();

                 

                because we know we won't load it from our domain.

                • 5. Re: Classloading and caching changes
                  alesj

                  So, once we determined what we need to do, this is how it's impled atm.

                   

                  A new CachedLoader interface was introduced

                   

                  public interface CacheLoader extends Loader
                  {
                     /**
                      * Check the class cache.
                      *
                      * @param classLoader the reference classloader (possibly null)
                      * @param name the name of the class
                      * @param path the path of the class resource
                      * @param allExports whether to look at all exports
                      * @return the class if cached
                      */
                     Class<?> checkClassCache(BaseClassLoader classLoader, String name, String path, boolean allExports);
                  }

                   

                  where the BaseClassLoaderDomain does before and after checks

                   

                     public Class<?> checkClassCache(BaseClassLoader classLoader, String name, String path, boolean allExports)
                     {
                        Class<?> result = checkCacheBefore(classLoader, name, path, allExports);
                        if (result != null)
                           return result;
                  
                        result = checkCacheAfter(classLoader, name, path, allExports);
                        if (result != null)
                           return result;
                  
                        result = checkClassCacheLocally(classLoader, name, path, allExports);
                        if (result != null)
                           return result;
                  
                        return null;
                     }
                  

                   

                  where the actual impl lives in ClassLoaderDomain

                   

                  protected Class<?> checkCacheBefore(BaseClassLoader classLoader, String name, String path, boolean allExports)
                     {
                        if (parent == null || parent instanceof CacheLoader == false)
                           return null;
                  
                        ClassFilter filter = getParentPolicy().getBeforeFilter();
                        if (filter.matchesClassName(name))
                        {
                           CacheLoader loader = (CacheLoader) parent;
                           return loader.checkClassCache(classLoader, name, path, allExports);
                        }
                        return null;
                     }
                  
                     /**
                      * Only check parent after if we already blacklisted this resource.
                      */
                     @Override
                     protected Class<?> checkCacheAfter(BaseClassLoader classLoader, String name, String path, boolean allExports)
                     {
                        if (parent == null || parent instanceof CacheLoader == false || isBlackListedClass(path) == false)
                           return null;
                  
                        ClassFilter filter = getParentPolicy().getAfterFilter();
                        if (filter.matchesClassName(name))
                        {
                           CacheLoader loader = (CacheLoader) parent;
                           return loader.checkClassCache(classLoader, name, path, allExports);
                        }
                        return null;
                     }
                  

                   

                  The 3 current CacheLoader impls are

                  * BaseClassLoaderDomain (we already saw its impl)

                  * BaseDelegateLoader (it just delegates to its domain)

                  * ClassLoaderToLoaderAdapter (see below)

                   

                    public Class<?> checkClassCache(BaseClassLoader bcl, String name, String path, boolean allExports)
                     {
                        if (findLoadedClass == null)
                           return null;
                  
                        final ClassLoader classLoader = getClassLoader();
                        try
                        {
                           return (Class<?>) findLoadedClass.invoke(classLoader, name);
                        }
                        catch (Exception e)
                        {
                           log.warn("Unexpected error retrieving found class " + name + " from classloader " + classLoader, e);
                           return null;
                        }
                     }
                  

                   

                  I have also hacked a simple debug-able mock in Deployers project -- where we can check different deployment types

                  * http://anonsvn.jboss.org/repos/jbossas/projects/jboss-deployers/trunk/deployers-vfs/src/test/java/org/jboss/test/deployers/vfs/classloading/test/ClassLoaderCachingTestCase.java