| Summary: | WebappClassLoader.loadClass synchronization issue due to coarse lock at WebappClassLoader instance level instead of been by class name | ||
|---|---|---|---|
| Product: | Tomcat 8 | Reporter: | Sebastien Tardif <SebTardif> |
| Component: | Catalina | Assignee: | Tomcat Developers Mailing List <dev> |
| Status: | RESOLVED FIXED | ||
| Severity: | enhancement | CC: | koturano, mlukica, SebTardif |
| Priority: | P2 | ||
| Version: | 8.0.x-trunk | ||
| Target Milestone: | ---- | ||
| Hardware: | PC | ||
| OS: | All | ||
|
Description
Sebastien Tardif
2014-05-15 16:38:31 UTC
The same exact application didn't have the issue with GlassFish 3.x latest. The same issues happen to others -> https://groups.google.com/forum/#!topic/optiq-dev/amCEv1psDrI The partial stack trace is largely useless since it does not show what the critical thread (http-bio-31680-exec-41) is doing. When considering any proposed changes, keep this in mind: http://svn.apache.org/viewvc?view=revision&revision=927565 Thanks for the link. It does mentions for an attempt to provide parallel classloading for Tomcat 7 but that doesn't seem to have happened. http-bio-31680-exec-41 is running loading a class. Any of the stack below will come to that state at some point. So you can pick anyone as a representative of http-bio-31680-exec-41. My application does work, it's just 15x slower than with GlassFish. The thread dump was provided to show that we do have different code doing class lookup, about different classes, all waiting. The code triggering loading is not custom but JDK JAXB. My understanding is that the problem is well understood, so a use case is not adding anything here. Everybody know it's creating problems, that doesn't look good, but it have been declared too hard to fix, and so progress stopped 4 years ago. Previous deadlock issues may have been related to Java JDK version before 7 -> http://docs.oracle.com/javase/7/docs/technotes/guides/lang/cl-mt.html Optimized class loader -> https://github.com/jboss-modules/jboss-modules/blob/master/src/main/java/org/jboss/modules/ConcurrentClassLoader.java Java 7 was release July 28, 2011. The implementation of GlassFish "working" on JDK 6, is avoiding "systematic" synchronization on the instance, and so that seems to have been enough to be fast, see http://grepcode.com/file/repo1.maven.org/maven2/org.glassfish.web/war-util/3.0/org/glassfish/web/loader/WebappClassLoader.java In other words, they have at least tried to synchronized the smaller block as possible in all cases, and also with dubious construct, used some optimization similar to read/write lock. Again, please provide the full stack trace. It might tell us something it doesn't tell you. Tomcat 7 has to run on Java 6 and experience to date has been that synchronizing on anything other than the method leads to problems for a small number if users. Tomcat 8 is where there is scope to revisit parallel loading. I've done some refactoring of WebappClassLoader and there is now also a ParallelWebappClassLoader that uses a lock based on the class name. This will be included in 8.0.13 onward. Just some documentation notes. As a reference about this new feature.
1. This feature is off by default.
2. To enable it, add the following XML element
either to the context file of your web application,
or to the global conf/context.xml (to enable this for all webapps by default):
<Loader loaderClass="org.apache.catalina.loader.ParallelWebappClassLoader" />
It works for me with JDK 7u67.
Once 8.0.13 (or later) is officially released, the documentation will be at
http://tomcat.apache.org/tomcat-8.0-doc/config/loader.html
3. MBeans for the new class are visible in jconsole application, under Catalina/ParallelWebappClassloader. An ObjectName looks like
Catalina:type=ParallelWebappClassLoader,host=localhost,context=/examples
Still not sure the problem is solved. The problem tested in apache-tomcat-8.0.18 with ParallelWebappClassLoader loader. And apache-tomcat-7.0.35 has much better performance. Do you have unit tests to check where is a bottleneck? or there is better idea how to fix the problem with slow classloading? JConsole shows that org.apache.catalina.loader.WebappClassLoaderBase.loadClass is too slow in apache-tomcat-8.0.18: Name: http-nio-8080-exec-20 State: BLOCKED on org.apache.catalina.loader.WebappClassLoader@39d950df owned by: http-nio-8080-exec-80 Total blocked: 2 402 Total waited: 20 Stack trace: org.apache.catalina.loader.WebappClassLoaderBase.loadClass(WebappClassLoaderBase.java:1190) org.apache.catalina.loader.WebappClassLoaderBase.loadClass(WebappClassLoaderBase.java:1157) javax.el.ImportHandler.findClass(ImportHandler.java:196) javax.el.ImportHandler.resolveClass(ImportHandler.java:169) javax.servlet.jsp.el.ScopedAttributeELResolver.getValue(ScopedAttributeELResolver.java:62) org.apache.jasper.el.JasperELResolver.getValue(JasperELResolver.java:110) org.apache.el.parser.AstIdentifier.getValue(AstIdentifier.java:80) org.apache.el.parser.AstEmpty.getValue(AstEmpty.java:46) org.apache.el.parser.AstNot.getValue(AstNot.java:43) org.apache.el.parser.AstAnd.getValue(AstAnd.java:37) org.apache.el.parser.AstOr.getValue(AstOr.java:42) org.apache.el.ValueExpressionImpl.getValue(ValueExpressionImpl.java:184) org.apache.jasper.runtime.PageContextImpl.proprietaryEvaluate(PageContextImpl.java:936) org.apache.jsp.jsp.modules.category.breadCrumbs_jsp._jspx_meth_c_005fif_005f2(breadCrumbs_jsp.java:848) org.apache.jsp.jsp.modules.category.breadCrumbs_jsp._jspx_meth_v_005flist_005f0(breadCrumbs_jsp.java:817) org.apache.jsp.jsp.modules.category.breadCrumbs_jsp._jspService(breadCrumbs_jsp.java:221) org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:70) The stack trace shows you aren't using parallel class loading. The lock is on the ClassLoader rather than a class specific object. The problem occurred in eclipse and context.xml was not loaded properly. Right now everything works good. Thanks for the help. Hi, is it possible to port this change to Tomcat 7 in /tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java ? No. It requires Java 7 and Tomcat 7 has to run on Java 6. (In reply to Mark Thomas from comment #13) > No. It requires Java 7 and Tomcat 7 has to run on Java 6. Could we wrap some of this capability in Jre7Compat.isSupported()? Lots of folks are running Tomcat 7 on Java 7+. Java 6 is EOL so ... pretty much everybody should have moved up by now. Oracle's public support has ended for Java 6 but other vendors may have different policies and if you pay Oracle $ you can still get support, updates etc. (and lots of companies do). Using JreCompat looks doable. The two key questions would be how invasive is the patch and what is the impact on performance. *** Bug 57681 has been marked as a duplicate of this bug. *** Please consider not only the performance impact but also stability of web application. I am seeing 2-10 sec pauses for all requests in my web application caused by some bad code in just one request. The performance hit would only occur during class loading. By the time a few requests have been processed, class loading should have settled-down quite a bit. Some thoughts about performance: 1. This is mostly introducing a single "if" and a test against a static final field (Jre7Compat.forLanguageTagMethod). The JIT will likely inline that method. 2. If the branch is not tolerable at runtime, perhaps the branch can be executed a single time to install a delegate adapter, something like this: WebappClassLoader: public static Java7 extends WebappClassLoader // etc, do Java 7 stuff public static Java6 extends WebappClassLoader // etc, do Java 6 stuff <init>: if(JreCompat.getInstance().isJre7Supported()) delegate = new Java7ClassLoader(); else delegate = new Java6ClassLoader(); loadClass(): return delegate.loadClass(); Or, if you don't want to use Java method dispatch because delegation is too slow, you can use WebappClassLoader like a factory like this: WebappClassLoader: getInstance(): return JreCompat.getInstance().isJre7Supported() ? new Java7() : new Java6(); Really, synchronization is going to dominate the performance equation here and not the presence (or absence) of a trivial null-check. (In reply to Christopher Schultz from comment #18) > > 2. If the branch is not tolerable at runtime, perhaps the branch can be > executed a single time to install a delegate adapter, something like this: > This cannot be solved with an adapter that is installed atop the current WebappClassLoader class. See [1]. The class itself and all of its parent classes must be registered as parallel-capable. This needs 1. Refactoring, involving extraction of common parent class. 2. Implementation of alternative class loader class that is parallel-capable. The default implementation (WebappClassLoader) must stay with being non parallel capable. The common parent class must be registered as parallel-capable. If ClassLoader.registerAsParallelCapable() is called via reflection, I wonder whether the registerAsParallelCapable() method can correctly determine the calling class. We are not calling the method directly, but via reflection APIs. It needs a proof of concept. 3. Testing, testing, testing... [1] http://docs.oracle.com/javase/7/docs/api/java/lang/ClassLoader.html#registerAsParallelCapable%28%29 (In reply to Konstantin Kolinko from comment #19) > (In reply to Christopher Schultz from comment #18) > > > > 2. If the branch is not tolerable at runtime, perhaps the branch can be > > executed a single time to install a delegate adapter, something like this: > > > > This cannot be solved with an adapter that is installed atop the current > WebappClassLoader class. > > See [1]. The class itself and all of its parent classes must be registered > as parallel-capable. Aah, yes; I had forgotten about that. Well, that makes things ugly. > This needs > 1. Refactoring, involving extraction of common parent class. > > 2. Implementation of alternative class loader class that is parallel-capable. > > The default implementation (WebappClassLoader) must stay with being non > parallel capable. The common parent class must be registered as > parallel-capable. > > If ClassLoader.registerAsParallelCapable() is called via reflection, I > wonder whether the registerAsParallelCapable() method can correctly > determine the calling class. We are not calling the method directly, but via > reflection APIs. It needs a proof of concept. If registerAsParallelCapable can't be called via reflection, then it means that both Java 6 and Java 7 are required to build Tomcat 7. We already require Java 7 for building the (optional) WebSocket components. Could this simply be added to that list? > 3. Testing, testing, testing... Since the "regular" WebappClassLoader is still available (and the default CL), I think it's okay to mark it as an experimental feature and allow users to try it out. Is there a use case were WebappClassLoader must not be parallel? Is it an option to re-implement jdk1.7 method java.lang.ClassLoader#getClassLoadingLock(String) directly in WebappClassLoader for Tomcat7.0? I attached a patch that solves my Tomcat7.0 problem with waiting on lock for 3-10 sec in https://bz.apache.org/bugzilla/show_bug.cgi?id=57681 (In reply to Alex Koturanov from comment #21) > Is there a use case were WebappClassLoader must not be parallel? Parallel class loading has proved to be unreliable in the past. Until the stability of parallel loading is demonstrated by experience, serial loading needs to remain the default. > Is it an option to re-implement jdk1.7 method > java.lang.ClassLoader#getClassLoadingLock(String) directly in > WebappClassLoader for Tomcat7.0? No. > I attached a patch that solves my Tomcat7.0 problem with waiting on lock for > 3-10 sec in https://bz.apache.org/bugzilla/show_bug.cgi?id=57681 As I commented on that patch, that won't work. (In reply to Christopher Schultz from comment #20) > (In reply to Konstantin Kolinko from comment #19) > > This needs > > 1. Refactoring, involving extraction of common parent class. > > > > 2. Implementation of alternative class loader class that is parallel-capable. > > > > The default implementation (WebappClassLoader) must stay with being non > > parallel capable. The common parent class must be registered as > > parallel-capable. > > > > If ClassLoader.registerAsParallelCapable() is called via reflection, I > > wonder whether the registerAsParallelCapable() method can correctly > > determine the calling class. We are not calling the method directly, but via > > reflection APIs. It needs a proof of concept. > > If registerAsParallelCapable can't be called via reflection, then it means > that both Java 6 and Java 7 are required to build Tomcat 7. If we are going to have to play reflection tricks then we might was well go all the way, change the access permissions and call ParallelLoaders.register() directly. > We already require Java 7 for building the (optional) WebSocket components. > Could this simply be added to that list? I'd really rather not. WebSocket is optional. The web application class loader is a little more fundamental. > > 3. Testing, testing, testing... > > Since the "regular" WebappClassLoader is still available (and the default > CL), I think it's okay to mark it as an experimental feature and allow users > to try it out. No objections to that. With the usual caveats about minimal impact on the existing implementation of course. (In reply to Mark Thomas from comment #22) > (In reply to Alex Koturanov from comment #21) > > I attached a patch that solves my Tomcat7.0 problem with waiting on lock for > > 3-10 sec in https://bz.apache.org/bugzilla/show_bug.cgi?id=57681 > > As I commented on that patch, that won't work. A bit more details would be very helpful. We'd recently migrated from Weblogic 11g to Tomcat 8/Java 8. We very quickly found this to be a critical problem as well in highly concurrent environment. Glad to have come across it and have the remediation already available. Curious as to why this wouldn't (at least at some point) be made the default? |