You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Alex Cruikshank <al...@epitonic.com> on 2000/02/22 22:14:43 UTC
[PATCH]: significant performance boost for JSP pages
I apologize in advance for the long email.
I was profiling a JSP application running on the 3.1M1 server, and I
noticed that my JSP pages were taking much longer (by at least 2 orders of
magnitude) than the normal servlets. I could see that the service method
of the JSP page was taking less than a millisecond to execute, so the bulk
of the time must be taken up by the JSP request handling.
It turns out that the offending code is the getClassName() method in the
org.apache.jasper.compiler.ClassName class. This class seems to be parsing
the actual class data to read the name of a class to get the current class
number. This turns out to be extraordinarily expensive; taking 10-200
milliseconds both on our top of the line e250 SUN machine and a Compaq
Proliant production ready Linux machine. The time it takes to run seems to
be dependent on the size and complexity of the JSP class.
This expense would be tolerable if the method were only called when a JSP
file is outdated and needs to be compiled (as the author seems to have
intended), but the current code calls the method twice every time a JSP
page is requested regardless of whether it needs to be compiled. What's
worse, the code called from a synchronized method, JspLoader.loadJSP(),
which means that every JSP page request will have wait its turn to execute
this code. For a moderate complexity JSP file that takes 200 milliseconds
for the getClassName() method, this sets a hard limit of 2.5 requests per
second for the entire server, which is unacceptable for even a relatively
low load web site. I observed this deadlock experimentally in my load tests.
Fortunately, this problem can be rather easily fixed without major
architectural changes. The problem lies in the fact that JspCompiler ends
up calling the getClassName() method twice in its computeClassFileData()
method and that this method is called in it's constructor. The
JspLoader.loadJSP() method uses an instance of JspCompiler (via the
JspEngineContext.getCompiler() method) to determine whether a JSP file
needs to be (re-)compiled, so it ends up executing the
computeClassFileData() method every time, even though the data generated by
it is used only a tiny fraction of the time. The solution I propose
involves spliting the code that determines whether a jsp file is outdated
into the isOutDated() method and making it public, and moving the
computeClassFileData() and computeJavaFileName() requests (which are needed
for compilation) out of the constructor and into their respective accessors
so the computation happens as needed. This also allows us to de-couple the
outdated test from the actual compilation, so we can synchronize only when
the class needs to be compiled (thus avoiding deadlock with no reduction in
safety).
A quick test on a test.jsp gave the following results (in milliseconds):
BEFORE:
[alex@duranduran tomcat]$ JspLoader.loadJSP() time: 3254
JspLoader.loadJSP() time: 77
JspLoader.loadJSP() time: 70
JspLoader.loadJSP() time: 68
JspLoader.loadJSP() time: 69
JspLoader.loadJSP() time: 111
JspLoader.loadJSP() time: 94
JspLoader.loadJSP() time: 68
JspLoader.loadJSP() time: 68
...
JspLoader.loadJSP() time: 69
JspLoader.loadJSP() time: 68
[alex@duranduran tomcat]$ touch webpages/test.jsp
[alex@duranduran tomcat]$ JspLoader.loadJSP() time: 854
JspLoader.loadJSP() time: 130
JspLoader.loadJSP() time: 70
JspLoader.loadJSP() time: 68
JspLoader.loadJSP() time: 68
JspLoader.loadJSP() time: 68
...
JspLoader.loadJSP() time: 68
JspLoader.loadJSP() time: 69
AFTER:
JspLoader.loadJSP() time: 2950
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 2
touch webpages/test.jsp
[alex@duranduran tomcat]$ JspLoader.loadJSP() time: 1562
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 14
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
JspLoader.loadJSP() time: 1
And the difference would be greater for a more complicated JSP file. I've
also seen the elimination of deadlock with these changes in my load tests,
but reproducing those for you would be too complicated.
These changes are basically a work around to a more fundamental problems
that the JspCompilation system hasn't been designed for performance, and
the Mangling scheme is clunky. IMHO, these things should be overhauled for
RC2 (as is stated more than once in the existing code comments). I've
tried to keep these changes minimal to keep the 3.1 server stable.
The following diffs are against the TOMCAT_31_M1_RC1 version of tomcat in CVS:
T_31_M1_RC1 JspLoader.java
cvs diff: warning: unrecognized response `cvs: setgroups: Operation not
permitted' from cvs server
Index: JspLoader.java
===================================================================
RCS file:
/home/cvspublic/jakarta-tomcat/src/share/org/apache/jasper/runtime/JspLoader
.java,v
retrieving revision 1.3
diff -u -r1.3 JspLoader.java
--- JspLoader.java 1999/12/21 12:34:31 1.3
+++ JspLoader.java 2000/02/22 20:03:18
@@ -1,5 +1,5 @@
/*
- * $Header:
/home/cvspublic/jakarta-tomcat/src/share/org/apache/jasper/runtime/JspLoader
.java,v 1.3 1
999/12/21 12:34:31 rubys Exp $
+ * $Header:
/home/cvs/jakarta-tomcat/src/share/org/apache/jasper/runtime/JspLoader.java,
v 1.3 1999/12
/21 12:34:31 rubys Exp $
* $Revision: 1.3 $
* $Date: 1999/12/21 12:34:31 $
*
@@ -207,10 +207,11 @@
* @param classpath explicitly set the JSP compilation path.
* @return true if JSP files is newer
*/
- public synchronized boolean loadJSP(String name, String classpath,
+ public boolean loadJSP(String name, String classpath,
boolean isErrorPage, HttpServletRequest req, HttpServletResponse res)
throws JasperException, FileNotFoundException
{
+long loadJSPTime = System.currentTimeMillis();
Class jspClass = (Class) loadedJSPs.get(name);
boolean firstTime = jspClass == null;
@@ -223,7 +224,14 @@
Compiler compiler = ctxt.createCompiler();
try {
- outDated = compiler.compile();
+ if ((jspClass == null) || (compiler.isOutDated() ))
+ {
+ synchronized ( this )
+ {
+ if ((jspClass == null) || (compiler.isOutDated() ))
+ outDated = compiler.compile();
+ }
+ }
} catch (FileNotFoundException ex) {
throw ex;
} catch (JasperException ex) {
@@ -244,6 +252,7 @@
loadedJSPs.put(name, jspClass);
}
+System.err.println( "JspLoader.loadJSP() time: " +
(System.currentTimeMillis() - loadJSPTime) );
return outDated;
}
AT_31_M1_RC1 Compiler.java
cvs diff: warning: unrecognized response `cvs: setgroups: Operation not
permitted' from cvs server
Index: Compiler.java
===================================================================
RCS file:
/home/cvspublic/jakarta-tomcat/src/share/org/apache/jasper/compiler/Compiler
.java,v
retrieving revision 1.6
diff -u -r1.6 Compiler.java
--- Compiler.java 2000/01/08 21:31:38 1.6
+++ Compiler.java 2000/02/22 20:03:31
@@ -1,7 +1,7 @@
/*
- * $Header:
/home/cvspublic/jakarta-tomcat/src/share/org/apache/jasper/compiler/Compiler
.java,v 1.6 2
000/01/08 21:31:38 rubys Exp $
- * $Revision: 1.6 $
- * $Date: 2000/01/08 21:31:38 $
+ * $Header:
/home/cvs/jakarta-tomcat/src/share/org/apache/jasper/compiler/Compiler.java,
v 1.7 2000/01
/21 04:17:21 rubys Exp $
+ * $Revision: 1.7 $
+ * $Date: 2000/01/21 04:17:21 $
*
* ====================================================================
*
@@ -230,7 +230,7 @@
* subclasses of Compiler. This is used by the compile method
* to do all the compilation.
*/
- protected abstract boolean isOutDated();
+ public abstract boolean isOutDated();
/**
* Set java compiler info
cvs diff: warning: unrecognized response `cvs: setgroups: Operation not
permitted' from cvs server
Index: JspCompiler.java
===================================================================
RCS file:
/home/cvspublic/jakarta-tomcat/src/share/org/apache/jasper/compiler/JspCompi
ler.java,v
retrieving revision 1.4
diff -u -r1.4 JspCompiler.java
--- JspCompiler.java 1999/12/28 16:25:54 1.4
+++ JspCompiler.java 2000/02/22 20:03:40
@@ -90,8 +90,8 @@
setMangler(this);
computePackageName();
computeClassFileName();
- computeClassFileData();
- computeJavaFileName();
+ // computeClassFileData();
+ // computeJavaFileName();
}
public final String getPackageName() {
@@ -99,10 +99,18 @@
}
public final String getClassName() {
+ if ( className == null )
+ {
+ computeClassFileData();
+ }
return className;
}
public final String getJavaFileName() {
+ if ( javaFileName == null )
+ {
+ computeJavaFileName();
+ }
return javaFileName;
}
@@ -111,16 +119,6 @@
}
- /**
- * Return true if the .class file is outdated w.r.t
- * the JSP file.
- *
- * Can (meant to) be overridden by subclasses of JspCompiler.
- */
- protected boolean isOutDated() {
- return outDated;
- }
-
public static String [] keywords = {
"abstract", "boolean", "break", "byte",
"case", "catch", "char", "class",
@@ -184,7 +182,7 @@
}
public final void computeJavaFileName() {
- javaFileName = className + ".java";
+ javaFileName = getClassName() + ".java";
if (outputDir != null && !outputDir.equals(""))
javaFileName = outputDir + File.separatorChar + javaFileName;
}
@@ -251,8 +249,33 @@
}
+ /**
+ * Determines whether the current JSP class is older than the JSP file
from whence it came
+ */
+ public boolean isOutDated()
+ {
+ ServletContext ctx = ctxt.getServletContext();
+ File jspReal = null;
+
+ if (ctx != null)
+ jspReal = new File(ctx.getRealPath(jsp.getPath()));
+ else
+ jspReal = jsp;
+
+ File classFile = new File(classFileName);
+ if (classFile.exists())
+ {
+ outDated = classFile.lastModified() < jspReal.lastModified();
+ }
+ else
+ {
+ outDated = true;
+ }
+ return outDated;
+ }
+
private final void computeClassFileData()
- throws JasperException
+// throws JasperException
{
ServletContext ctx = ctxt.getServletContext();
@@ -278,14 +301,21 @@
} else {
cfd = new ClassFileData(outDated, classFileName, null);
}
- String classNameFromFile = ClassName.getClassName(classFileName);
- String cn = cfd.getClassName();
- int lastDot = cn.lastIndexOf('.');
- if (lastDot != -1)
- className = cn.substring(lastDot+1,
- classNameFromFile.length());
- else
- className = cn;
+ try
+ {
+ String classNameFromFile =
ClassName.getClassName(classFileName);
+ String cn = cfd.getClassName();
+ int lastDot = cn.lastIndexOf('.');
+ if (lastDot != -1)
+ className = cn.substring(lastDot+1,
+ classNameFromFile.length());
+ else
+ className = cn;
+ }
+ catch (JasperException ex)
+ {
+ ex.printStackTrace();
+ }
}
}
}
@@ -317,10 +347,6 @@
=
Integer.valueOf(className.substring(className.lastIndexOf(Constants.JSP_TOKEN)+
Constants.JSP_TOKEN.leng
Constants.JSP_TOKEN.length(),
className.length())).int
className.length())).intValue();
- }
-
- public boolean isOutDated() {
- return outDated;
}
public String getClassName() {
--------------------------
Alex Cruikshank
Senior Software Engineer
Epitonic.com
Re: [PATCH]: significant performance boost for JSP pages
Posted by Alex Cruikshank <al...@epitonic.com>.
At 04:32 PM 2/22/00 -0800, you wrote:
>Alex Cruikshank wrote:
> > [snipped description of JSP performance problems]
> > What's worse, the code called from a synchronized method,
> JspLoader.loadJSP(),
> > which means that every JSP page request will have wait its turn to execute
> > this code. [...]
>
>I'm pretty sure this method was made synchronized as a quick and dirty fix
>for a serious multithreading problem (two JSP pages accessed for the first
>time
>at roughly the same time ended up as servlets mixing content from the two
>pages). Besides removing "synchronized", the core problem must be solved.
I don't advocate removing the synchronization, but rather, increasing the
granularity of the synchronization block.
+ if ((jspClass == null) || (compiler.isOutDated() ))
+ {
+ synchronized ( this )
+ {
+ if ((jspClass == null) || (compiler.isOutDated() ))
+ outDated = compiler.compile();
+ }
+ }
This type of construct is impossible without making the isOutDated() method
public. The code should be equally thread safe, because the file is tested
before and after the synchronization lock. Synchronization can be
expensive by itself, and I feel some effort should be made to avoid it
where safe to do so regardless of the duration of the code within the
synchronization block.
- alex
>Hans
>--
>Hans Bergsten hans@gefionsoftware.com
>Gefion Software http://www.gefionsoftware.com
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: [PATCH]: significant performance boost for JSP pages
Posted by Hans Bergsten <ha...@gefionsoftware.com>.
Alex Cruikshank wrote:
> [snipped description of JSP performance problems]
> What's worse, the code called from a synchronized method, JspLoader.loadJSP(),
> which means that every JSP page request will have wait its turn to execute
> this code. [...]
I'm pretty sure this method was made synchronized as a quick and dirty fix
for a serious multithreading problem (two JSP pages accessed for the first time
at roughly the same time ended up as servlets mixing content from the two
pages). Besides removing "synchronized", the core problem must be solved.
Hans
--
Hans Bergsten hans@gefionsoftware.com
Gefion Software http://www.gefionsoftware.com