You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Werner Punz <we...@gmail.com> on 2009/08/14 15:18:47 UTC
myfaces classloaders
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
Re: myfaces classloaders
Posted by Werner Punz <we...@gmail.com>.
Ok the good news is, I hacked it in semi dirty into the latest trunk in
myfaces and things work out pretty well, I have my groovy example
running without a custom classloader.
So adding this chain definitely is a way to go.
The question is, does it also really help on the OSGI side, if yes
I would say we should add it to myfaces 1.2 as well.
Werner
Werner Punz schrieb:
> 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 <we...@gmail.com>:
>>> 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
>>>
>>>
>>
>
>
Re: myfaces classloaders
Posted by Werner Punz <we...@gmail.com>.
That does not work out with scripting extensions...
The issue is if I want to proved a factory which is
scripted then I have to provide the classloader upfront!
I am not sure how this works out with osgi either.
Artifacts are loaded and provided by a cloassloader not vice versa :-(
This only can work in the domain of java itself where you can do a
classpath scanning for artefacts providing the classloader information.
Werner
Bernhard Huemer schrieb:
> 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 <we...@gmail.com>:
>>>> 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
>>>>
>>>>
>>>
>>
>>
>
>
Re: myfaces classloaders
Posted by Werner Punz <we...@gmail.com>.
Werner Punz schrieb:
> the approach that an artefact should provide its own classloader
> information is not possible in the scope of scripted extensions.
> Secondly there is not the metadata even on the jsf side to allow that,
> unless you scan for special implementation details outside of the spec.
> Heck even on the java side you have to load the class first to get the
> classloader information for the class!
> The only thing possible would be some autoscan for classloaders in the
> classpath which have to be added to the chain!
>
Actually on a second thought, I like the idea of being able to hotplug
a new loader provided by a library this would reduce configuration. It
still is impossible
to provide it on artefact level though :-(
In either way I would propose following.
First make it the way via configuration that way we can provide
the functionality relatively easy, later add autoscanning
I think this needs further discussion.
I would like to add the changes to the 1.2 trunk codebase though because
this is my current development build for the groovy stuff.
I think they are not very big, so it is not really critical to provide them.
We still can refactor that later on.
That way I can provide a working groovy extension as soon as I am able
to commit my code into the new project out of the box.
(Sorry for the change of mind in this case I just want to have people
testing the stuff asap)
Re: myfaces classloaders
Posted by Werner Punz <we...@gmail.com>.
Bernhard Huemer schrieb:
> 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!
>
We are talking about several issues here
first of all you want to provide something along the lines of
viewHandler coded in groovy has to provide the classloader it has to be
loaded from, here we have a hen and egg problem a groovy file awaits the
code so that it can be loaded and the loader which has to interprete the
file has to come from the groovy file itself!
That does not work, the jar itself however could bundle its own
scriptlign loader which then would automatically added to the chain.
That theoretically would work, if we add classpath scanning as well (and
again slow down the startup in this area)
However the approach that an artefact should provide its own classloader
information is not possible in the scope of scripted extensions.
Secondly there is not the metadata even on the jsf side to allow that,
unless you scan for special implementation details outside of the spec.
Heck even on the java side you have to load the class first to get the
classloader information for the class!
The only thing possible would be some autoscan for classloaders in the
classpath which have to be added to the chain!
Re: myfaces classloaders
Posted by Bernhard Huemer <be...@gmail.com>.
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 <we...@gmail.com>:
>>>> 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
>>>>
>>>>
>>>
>>
>>
>
>
Re: myfaces classloaders
Posted by Bernhard Huemer <be...@gmail.com>.
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 <we...@gmail.com>:
>>> 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
>>>
>>>
>>
>
>
Re: myfaces classloaders
Posted by Werner Punz <we...@gmail.com>.
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 <we...@gmail.com>:
>> 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
>>
>>
>
Re: myfaces classloaders
Posted by Bruno Aranda <br...@gmail.com>.
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 <we...@gmail.com>:
> 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
>
>