On 27/02/2008, Richard S. Hall <[EMAIL PROTECTED]> wrote: > > The "diagnoseClassLoadError()" method was always intended to be purely > for debugging purposes. It seems like it would be easier to just disable > it if the framework log level is not at least at DEBUG.
+1 if it was only meant for debugging, then that's the simplest fix this would mean a drop in performance when debugging, but that's 'business-as-usual' - and I expect any classloader issues would be solved well before an application/update hits a production server... -> richard > > > Guillaume Nodet wrote: > > I've raised a JIRA with a patch that should accomodate this problem. > > I must admit this is not the cleanest thing, but given a 95% > > performance boost in my case > > I'd be happy it it is included :-) > > See https://issues.apache.org/jira/browse/FELIX-500 > > > > On Mon, Feb 25, 2008 at 5:05 PM, Guillaume Nodet <[EMAIL PROTECTED]> > wrote: > > > >> I suppose we could make the ClassNotFoundException class a static > class, give it > >> a transient reference to the IModule and make sure the message is > extracted when > >> the Exception is serialized. > >> Thoughts ? > >> > >> > >> > >> On Mon, Feb 25, 2008 at 4:47 PM, Dale Peakall <[EMAIL PROTECTED]> > wrote: > >> > These patches do have the problem that exceptions are supposed to be > >> > Serializable - and IModule is not Serializable. I don't know if > the > >> > information necessary to construct the message is Serializable and > can > >> > be quickly extracted from the module definition - given the > apparent > >> > run-time cost of the method I suspect not. > >> > > >> > > >> > > >> > Stuart McCulloch wrote: > >> > > On 25/02/2008, Guillaume Nodet <[EMAIL PROTECTED]> wrote: > >> > > > >> > >> Thanks Stuart. The exception was catched and the getMessage > method > >> > >> called, so I've > >> > >> hacked a bit more, but the following patch seems to work great > for me. > >> > >> > >> > > > >> > > > >> > > cool, could you raise a JIRA issue and attach the combined patch > - thanks > >> > > > >> > > Index: > src/main/java/org/apache/felix/moduleloader/ModuleImpl.java > >> > > > >> > >> > =================================================================== > >> > >> --- src/main/java/org/apache/felix/moduleloader/ModuleImpl.java > >> > >> (revision 630863) > >> > >> +++ src/main/java/org/apache/felix/moduleloader/ModuleImpl.java > (working > >> > >> copy) > >> > >> @@ -147,10 +147,17 @@ > >> > >> } > >> > >> catch (ClassNotFoundException ex) > >> > >> { > >> > >> - m_logger.log( > >> > >> - Logger.LOG_WARNING, > >> > >> - ex.getMessage(), > >> > >> - ex); > >> > >> + // Diagnosing the class loader error is very > costly, so only > >> > >> + // perform this call (indirectly through > ex.getMessage()) > >> > >> + // if necessary. > >> > >> + // See R4SearchPolicyCore#findClass > >> > >> + if (m_logger.getLogLevel() >= Logger.LOG_WARNING) > >> > >> + { > >> > >> + m_logger.log( > >> > >> + Logger.LOG_WARNING, > >> > >> + ex.getMessage(), > >> > >> + ex); > >> > >> + } > >> > >> } > >> > >> return null; > >> > >> > >> > >> } > >> > >> Index: > >> > >> > src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java > >> > >> > =================================================================== > >> > >> --- > >> > >> > src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java > >> > >> > >> > >> (revision 630863) > >> > >> > >> > >> +++ > >> > >> > src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java > >> > >> (working copy) > >> > >> @@ -178,7 +178,7 @@ > >> > >> return null; > >> > >> } > >> > >> > >> > >> - public Class findClass(IModule module, String name) > >> > >> + public Class findClass(final IModule module, final String > name) > >> > >> throws ClassNotFoundException > >> > >> { > >> > >> try > >> > >> > >> > >> @@ -191,8 +191,13 @@ > >> > >> > >> > >> } > >> > >> catch (ClassNotFoundException ex) > >> > >> { > >> > >> - String msg = diagnoseClassLoadError(module, name); > >> > >> - throw new ClassNotFoundException(msg, ex); > >> > >> > >> > >> + throw new ClassNotFoundException("", ex) > >> > >> > >> > >> + { > >> > >> + public String getMessage() > >> > >> + { > >> > >> + return diagnoseClassLoadError(module, > name); > >> > >> + } > >> > >> + }; > >> > >> } > >> > >> > >> > >> // We should never reach this point. > >> > >> > >> > >> > >> > >> > >> > >> On Mon, Feb 25, 2008 at 3:37 PM, Stuart McCulloch > >> > >> <[EMAIL PROTECTED]> wrote: > >> > >> > >> > >>> On 25/02/2008, Guillaume Nodet <[EMAIL PROTECTED]> wrote: > >> > >>> > > >> > >>> > I'm currently working on ServiceMix 4 which uses Felix.\ > >> > >>> > > >> > >>> > I have some really important performance problems in > classloading. > >> > >>> > When loading JBI applications (they have some very specific > >> > >>> > classloading architecture > >> > >>> > that must be implemented to be JBI compliant), 95% of the > time is > >> > >>> > spent in the following method: > >> > >>> > R4SearchPolicyCore.diagnoseClassLoadError() > >> > >>> > which is not really acceptable. > >> > >>> > > >> > >>> > The problem is that the classloader is built dynamically and > does not > >> > >>> > completely rely on > >> > >>> > OSGi. For this reason, the classloader of JBI artifacts > delegates to > >> > >>> > the parent classloader, > >> > >>> > then looks inside its own jar. This means there will be > lots of > >> > >>> > ClassNotFoundException thrown > >> > >>> > by the parents classloader (ultimately by Felix). > >> > >>> > > >> > >>> > Is there any way to maybe speed / disable the > diagnoseClassLoadError > >> > >>> > method call which > >> > >>> > is purely informative ? > >> > >>> > >> > >>> > >> > >>> we could try a lazy-load approach (ie. only construct the > string in the > >> > >>> getMessage method) > >> > >>> for example, you might want to see if the following patch > helps, based > >> > >>> > >> > >> on > >> > >> > >> > >>> the current trunk: > >> > >>> > >> > >>> Index: > >> > >>> > > >> > >>> > >> src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java > > >> > >>> > >> =================================================================== > >> > >>> --- > >> > >>> > > >> > >>> > >> src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java > >> > >>> (revision 630859) > >> > >>> +++ > >> > >>> > > >> > >>> > >> src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java > >> > >>> (working copy) > >> > >>> @@ -178,7 +178,7 @@ > >> > >>> return null; > >> > >>> } > >> > >>> > >> > >>> - public Class findClass(IModule module, String name) > >> > >>> + public Class findClass(final IModule module, final String > name) > >> > >>> throws ClassNotFoundException > >> > >>> { > >> > >>> try > >> > >>> @@ -191,8 +191,14 @@ > >> > >>> } > >> > >>> catch (ClassNotFoundException ex) > >> > >>> { > >> > >>> - String msg = diagnoseClassLoadError(module, > name); > >> > >>> - throw new ClassNotFoundException(msg, ex); > >> > >>> + // lazy construction of exception message > >> > >>> + throw new ClassNotFoundException("", ex) > >> > >>> + { > >> > >>> + public String getMessage() > >> > >>> + { > >> > >>> + return diagnoseClassLoadError(module, > name); > >> > >>> + } > >> > >>> + }; > >> > >>> } > >> > >>> > >> > >>> // We should never reach this point. > >> > >>> @@ -3272,4 +3278,4 @@ > >> > >>> > >> > >>> return sb.toString(); > >> > >>> } > >> > >>> -} > >> > >>> \ No newline at end of file > >> > >>> +} > >> > >>> > >> > >>> although lazy construction might cause problems if people hold > onto > >> > >>> the exception for a long time, but I don't think this is > usually the > >> > >>> > >> > >> case > >> > >> > >> > >>> > >> > >>> I know the design of the JBI classloaders is not really a good > fit in > >> > >>> > OSGi, so I will investigate > >> > >>> > a better solution (leveraging more of OSGi classloaders) > anyway, but > >> > >>> > >> > >> I > >> > >> > >> > >>> > still wanted to report > >> > >>> > the problem. > >> > >>> > > >> > >>> > > >> > >>> > -- > >> > >>> > Cheers, > >> > >>> > Guillaume Nodet > >> > >>> > ------------------------ > >> > >>> > Blog: http://gnodet.blogspot.com/ > >> > >>> > > >> > >>> > >> > >>> > >> > >>> > >> > >>> -- > >> > >>> Cheers, Stuart > >> > >>> > >> > >>> > >> > >> > >> > >> > >> > >> -- > >> > >> > >> > >> Cheers, > >> > >> Guillaume Nodet > >> > >> ------------------------ > >> > >> Blog: http://gnodet.blogspot.com/ > >> > >> > >> > >> > >> > > > >> > > > >> > > > >> > > > >> > > >> > > >> > >> > >> > >> -- > >> > >> > >> Cheers, > >> Guillaume Nodet > >> ------------------------ > >> Blog: http://gnodet.blogspot.com/ > >> > >> > > > > > > > > > -- Cheers, Stuart
