Re: myfaces classloaders

Fri, 14 Aug 2009 08:31:09 -0700 

To clarify that a little bit more:
I'm more or less suggesting to introduce class loader extensions as well, but I don't think that they should be stored centrally. Instead I think each artifact should be able to provide the class loader extension! 

Bernhard Huemer wrote:

Personally I would prefer it if the artifact itself is responsible for 
providing the corresponding class loader instead of using a "try & error 
approach". This means that we would have to rewrite the configuration 
parts of MyFaces so that not only the class name but also the class 
loader to use are stored.



Note that, if I'm referring to a class loader, I don't necessarily mean 
the java.util.ClassLoader, i.e. we could introduce an own ClassLoader 
interface with different implementations like for example 
"ThreadContextClassLoader" (a class that provides the current behaviour 
by obtaining the webapp class loader at first), a "GroovyClassLoader" or 
an "OsgiClassLoader", etc:


///

public interface ClassLoader {
  public Class<?> loadClass(String className);
}

public class ThreadContextClassLoader implements ClassLoader {
  public Class<?> loadClass(String className) {
    ClassLoader classLoader = getContextClassLoader();
    if (classLoader == null) {
      classLoader = ThreadContextClassLoader.class.getClassLoader();
    }

    return Class.forName(className, false, classLoader);
  }
}

// ...

\\\


In doing so we would still maintain the boundaries of each Classloader. 
I think otherwise it can be quite difficult to determine which class 
loader has instantiated a certain JSF artifact.


regards,
Bernhard


P.S. I offer to rewrite the configuration parts of MyFaces (I've already 
done that before locally for the purpose of integrating OSGi)!


Werner Punz wrote:

Well if osgi needs a different classloader which at some stages is not 
the context classloader the current classloader then yes

this would help.

Werner


Bruno Aranda schrieb:

I agree if I understand it correctly. This would help as well to deal
with the classloading stuff for OSGi,

Cheers,

Bruno

2009/8/14 Werner Punz <[email protected]>:

Hia, while I am still preparing my groovy stuff I digged through
the myfaces class loading code.


Here is my problem, I currently use a custom classloader which roots 
into
the groovy code and if a file is present loads the class dynamically 
if not

it loads the class via the classloader currently set.

This mechanism is needed to be able to load the groovy artefacts from
various parts of jsf like view handlers, managed beans etc...

However this is messy, adding a classloader to a webapp is a no go.

I noticed that due to an issue regarding webapp startups (ear 
containers
change classloaders on the fly so you cannot rely on the context 
classloader
alone) all the forName code already has a centralized loading 
location in

place for artefacts.

To be able to deal with this problem in a clean way I would propose
following.
We should change the pattern of the classloading code to a chain of
responsibility pattern which means instead of:

  public static Class classForName(String type)
       throws ClassNotFoundException
   {



       if (type == null) throw new NullPointerException("type");
       try
       {
           // Try WebApp ClassLoader first
           return Class.forName(type,
                                false, // do not initialize for faster
startup
                                getContextClassLoader());
       }
       catch (ClassNotFoundException ignore)
       {

           // fallback: Try ClassLoader for ClassUtils (i.e. the 
myfaces.jar

lib)
           return Class.forName(type,
                                false, // do not initialize for faster
startup
                                ClassUtils.class.getClassLoader());
       }
   }


we should do it the following way
  public static Class classForName(String type)
       throws ClassNotFoundException
   {



       if (type == null) throw new NullPointerException("type");
       for(ClassLoaderExtension extension: classLoadingExtensions) {
           Class retVal = extension.forName(type);
           if(retVal != null) {
               return retVal;
           }

       }
       throw new ClassNotFoundException(name);
}


The main issue is all the existing methods are static so we
have to add the datastructures as well in a static way
(and probably we wont need a thread safety as well so we can
get a better performance for not doing it synchronized)


With the core logic of forName being distributed over several chain 
objects


And if we have a lot of those forName calls we might have a small
probably neglectable performance impact (might be fixable if we go
from arraylists to real arrays which are on assembler level cause
less operations).

This method would enable to plug in other loading mechanisms without
having to change the context classloader.


The other thing is, we need some kind of init code of the startup 
servlet

context which allows to setup such custom loaders.

along the lines of
_servletContext.getInitParam("org.apache.myfaces.CustomLoaders");


I will try to prototype all this with the current myfaces trunk.
If all goes well and I can eliminate the classloader we probably should
add those extensions to myfaces 2.0.


I personally like this path because it would allow us to hook in 
several

scripting engines in the long run without having to revert to

custom classloaders which are a pain in various container 
configurations.



Anyway what is your opinion about those changes?


Werner



































					Reply via email to