You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Kin-Man Chung <Ki...@Eng.Sun.COM> on 2002/05/08 22:44:10 UTC
Re: [PATCH] Re: [PROPOSAL] Modification of the code generated by
Jasper2
Denis,
I just had a chance to look at your patch, and I have couple of questions.
In the generated method finallies, there is the code fragment,
if (bitmask.get(1)) {
if (((javax.servlet.jsp.tagext.BodyTag)tags.elementAt(1)).doAfterBody() !=
javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUDE)
out = pageContext.popBody();
}
Shouldn't that be doStartTag() instead of doAfterBody()?
Also, if an exception occurs after the method is invoked, wouldn't it be
invoked again in finallies? I think you need to pass the return value for
the method to fanallies instead.
> Date: Mon, 29 Apr 2002 21:51:47 -0400 (EDT)
> From: Denis Benoit <be...@sympatico.ca>
> Subject: [PATCH] Re: [PROPOSAL] Modification of the code generated by Jasper2
> To: Tomcat Developers List <to...@jakarta.apache.org>
> MIME-version: 1.0
> Delivered-to: mailing list tomcat-dev@jakarta.apache.org
> Mailing-List: contact tomcat-dev-help@jakarta.apache.org; run by ezmlm
> X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N
> List-Post: <ma...@jakarta.apache.org>
> List-Subscribe: <ma...@jakarta.apache.org>
> List-Unsubscribe: <ma...@jakarta.apache.org>
> List-Help: <ma...@jakarta.apache.org>
> List-Id: "Tomcat Developers List" <tomcat-dev.jakarta.apache.org>
>
> On Mon, 29 Apr 2002, Remy Maucherat wrote:
>
> > This looks like a good idea to me (Kin-Man is not there this week, so it's
> > not an expert opinion). I would see that kind of change going into Jasper 2,
> > though. Do you think you can prepare a patch against that version ?
> >
> > Remy
> >
> >
> > --
> > To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> > For additional commands, e-mail: <ma...@jakarta.apache.org>
> >
>
> Thanks!
>
> We've done some benchmarks with JMeter, even though the case we tested is
pathologic,
> the JSP contained 100 tags! The results were impressive. The Jasper version
included
> with the "pre-beta" 4.1 tomcat averaged 20 seconds/hit, with the patch, the
CVS
> version of jasper2 average 0.8 second/hit. If there is less try/finally
nesting
> in the java code of the page, the difference is less impressive of course.
>
> The test setup, the test page and the detail of the results can be found:
>
> http://www3.sympatico.ca/benoitde/
>
> NOTE my "sunday patch" contained one bug, this one has been more tested.
>
> The patch against the CVS jasper2 is:
>
>
> Index:
jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/Generator.jav
a
> ===================================================================
> RCS file:
/home/cvspublic/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compil
er/Generator.java,v
> retrieving revision 1.6
> diff -u -r1.6 Generator.java
> ---
jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/Generator.jav
a 25 Apr 2002 18:16:06 -0000 1.6
> +++
jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/Generator.jav
a 30 Apr 2002 00:23:19 -0000
> @@ -63,6 +63,8 @@
> import java.util.*;
> import java.beans.*;
> import java.net.URLEncoder;
> +import java.io.ByteArrayOutputStream;
> +import java.io.PrintStream;
> import java.lang.reflect.Method;
> import javax.servlet.jsp.tagext.*;
> import org.xml.sax.Attributes;
> @@ -91,6 +93,9 @@
> private JspCompilationContext ctxt;
> private boolean breakAtLF;
> private PageInfo pageInfo;
> + private FinallyApplyer finallies;
> + private int tryBit;
> + private Stack tryStack;
>
> /**
> * @param s the input string
> @@ -176,6 +181,8 @@
> out.print ((String)iter.next());
> out.println(";");
> }
> + out.printil("import java.util.Vector;");
> + out.printil("import java.util.BitSet;");
> out.println();
>
> // Generate class declaration
> @@ -199,6 +206,17 @@
>
> // Constructor (empty so far) here
> // Other methods here
> + out.printil("private void addTagToVector(Vector tags, int index,
javax.servlet.jsp.tagext.Tag tag) {");
> + out.pushIndent();
> + out.printil("if (index + 1 > tags.size())");
> + out.pushIndent();
> + out.printil("tags.setSize(index + 1);");
> + out.popIndent();
> + out.printil("tags.setElementAt(tag, index);");
> + out.popIndent();
> + out.printil("}");
> + out.println();
> + out.println();
>
> // Now the service method
> out.printin("public void ");
> @@ -222,6 +240,8 @@
> out.printil("ServletConfig config = null;");
> out.printil("JspWriter out = null;");
> out.printil("Object page = this;");
> + out.printil("BitSet bitmask = new BitSet();");
> + out.printil("Vector tags = new Vector();");
>
> out.printil("try {");
> out.pushIndent();
> @@ -882,6 +902,10 @@
> out.println(" ---- */");
>
> Class tagHandlerClass = handlerInfo.getTagHandlerClass();
> +
> + boolean implementsTryCatchFinally =
> + TryCatchFinally.class.isAssignableFrom(tagHandlerClass);
> +
> out.printin(tagHandlerClass.getName());
> out.print(" ");
> out.print(tagHandlerVar);
> @@ -895,8 +919,22 @@
> declareTagVariableInfos(tagVarInfos, n.getTagData(),
> VariableInfo.AT_BEGIN);
>
> - out.printil("try {");
> - out.pushIndent();
> + if (implementsTryCatchFinally) {
> + out.printil("try {");
> + out.pushIndent();
> + } else {
> + out.printil("// try {");
> + out.printin("bitmask.set(");
> + Integer tryBitVal = new Integer(tryBit++);
> + tryStack.push(tryBitVal);
> + out.print(tryBitVal.toString());
> + out.println(");");
> + out.printin("addTagToVector(tags, ");
> + out.print(tryBitVal.toString());
> + out.print(", ");
> + out.print(tagHandlerVar);
> + out.println(");");
> + }
> out.printin("int ");
> out.print(tagEvalVar);
> out.print(" = ");
> @@ -918,8 +956,17 @@
> out.pushIndent();
>
> if (isBodyTag) {
> - out.printil("try {");
> - out.pushIndent();
> + out.printil("// try {");
> + out.printin("bitmask.set(");
> + Integer tryBitVal = new Integer(tryBit++);
> + tryStack.push(tryBitVal);
> + out.print(tryBitVal.toString());
> + out.println(");");
> + out.printin("addTagToVector(tags, ");
> + out.print(tryBitVal.toString());
> + out.print(", ");
> + out.print(tagHandlerVar);
> + out.println(");");
> out.printin("if (");
> out.print(tagEvalVar);
> out.println(" !=
javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUDE) {");
> @@ -981,19 +1028,35 @@
>
> if (n.getBody() != null) {
> if (implementsBodyTag) {
> - out.popIndent(); // try
> + Integer tryBitVal = (Integer)tryStack.pop();
> + out.printil("// } finally {");
> + out.printin("bitmask.clear(");
> + out.print(tryBitVal.toString());
> + out.println(");");
> +
> + out.printin("addTagToVector(tags, ");
> + out.print(tryBitVal.toString());
> + out.print(", ");
> + out.print(tagHandlerVar);
> + out.println(");");
>
> - out.printil("} finally {");
> - out.pushIndent();
> out.printin("if (");
> out.print(tagEvalVar);
> out.println(" !=
javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUDE)");
> out.pushIndent();
> out.printil("out = pageContext.popBody();");
> out.popIndent();
> +
> + finallies.beginPartMethod(tryBitVal.intValue());
> + finallies.print(" if (");
> +
finallies.print("((javax.servlet.jsp.tagext.BodyTag)tags.elementAt(");
> + finallies.print(tryBitVal.toString());
> + finallies.print(")).doAfterBody()");
> + finallies.println(" !=
javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUDE)");
> + finallies.println(" out = pageContext.popBody();");
>
> - out.popIndent();
> - out.printil("}");
> + finallies.endPartMethod();
> + out.printil("// }");
> }
>
> out.popIndent(); // EVAL_BODY
> @@ -1007,26 +1070,39 @@
> out.printil("return;");
> out.popIndent();
>
> - out.popIndent(); // try
> -
> // TryCatchFinally
> if (implementsTryCatchFinally) {
> + out.popIndent(); // try
> +
> out.printil("} catch (Throwable _jspx_exception) {");
> out.pushIndent();
> out.printin(tagHandlerVar);
> out.println(".doCatch(_jspx_exception);");
> out.popIndent();
> - }
> - out.printil("} finally {");
> - out.pushIndent();
> - if (implementsTryCatchFinally) {
> + out.printil("} finally {");
> + out.pushIndent();
> out.printin(tagHandlerVar);
> out.println(".doFinally();");
> + out.printin(tagHandlerVar);
> + out.println(".release();");
> + out.popIndent();
> + out.printil("}");
> + } else {
> + Integer tryBitVal = (Integer)tryStack.pop();
> + out.printil("// } finally {");
> + out.printin("bitmask.clear(");
> + out.print(tryBitVal.toString());
> + out.println(");");
> + out.printin(tagHandlerVar);
> + out.println(".release();");
> + out.printil("// }");
> + finallies.beginPartMethod(tryBitVal.intValue());
> +
finallies.printin("((javax.servlet.jsp.tagext.Tag)tags.elementAt(");
> + finallies.print(tryBitVal.toString());
> + finallies.print("))");
> + finallies.println(".release();");
> + finallies.endPartMethod();
> }
> - out.printin(tagHandlerVar);
> - out.println(".release();");
> - out.popIndent();
> - out.printil("}");
>
> // Declare and update AT_END variables
> updateVariableInfos(varInfos, VariableInfo.AT_END, true);
> @@ -1302,7 +1378,18 @@
> out.pushIndent();
>
> // Do stuff here for finally actions...
> +
> + out.printil("try {");
> + out.pushIndent();
> + out.printil("finallies(bitmask, out, tags, pageContext);");
> + out.popIndent();
> + out.printil("} catch (javax.servlet.jsp.JspException e) {");
> + out.pushIndent();
> + out.printil("if (pageContext != null)
pageContext.handlePageException(e);");
> + out.popIndent();
> + out.printil("}");
> out.printil("if (_jspxFactory != null)
_jspxFactory.releasePageContext(pageContext);");
> +
> out.popIndent();
> out.printil("}");
>
> @@ -1310,6 +1397,10 @@
> out.popIndent();
> out.printil("}");
>
> + // Call the final method
> + finallies.done();
> + out.printil(finallies.toString());
> +
> // Close the class definition
> out.popIndent();
> out.printil("}");
> @@ -1325,6 +1416,8 @@
> pageInfo = compiler.getPageInfo();
> beanInfo = pageInfo.getBeanRepository();
> breakAtLF = ctxt.getOptions().getMappedFile();
> + finallies = new FinallyApplyer();
> + tryStack = new Stack();
> }
>
> /**
> @@ -1417,6 +1510,55 @@
> */
> public Class getTagHandlerClass() {
> return tagHandlerClass;
> + }
> + }
> +
> + private static class FinallyApplyer {
> + private PrintStream finalOutput;
> + private ByteArrayOutputStream rawOutput;
> +
> + FinallyApplyer() {
> + rawOutput = new ByteArrayOutputStream();
> + finalOutput = new PrintStream(rawOutput, true);
> +
> + finalOutput.println();
> + finalOutput.println(" private void finallies(BitSet bitmask,
JspWriter out, Vector tags, PageContext pageContext)");
> + finalOutput.println(" throws javax.servlet.jsp.JspException {");
> + }
> +
> + public void done() {
> + finalOutput.println(" }");
> + }
> +
> + public void beginPartMethod(int bit) {
> + finalOutput.print(" if (bitmask.get(");
> + finalOutput.print(bit);
> + finalOutput.println(")) {");
> + }
> +
> + public void endPartMethod() {
> + finalOutput.println(" }");
> + finalOutput.println();
> + }
> +
> + public void println(String aLine) {
> + if (null != aLine) {
> + finalOutput.print(aLine);
> + }
> + finalOutput.println();
> + }
> +
> + public void printin(String partLine) {
> + finalOutput.print(" ");
> + finalOutput.print(partLine);
> + }
> +
> + public void print(String partLine) {
> + finalOutput.print(partLine);
> + }
> +
> + public String toString() {
> + return rawOutput.toString();
> }
> }
> }
>
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [PATCH] Re: [PROPOSAL] Modification of the code generated by
Jasper2
Posted by Denis Benoit <be...@sympatico.ca>.
Mr Chung,
On Wed, 8 May 2002, Kin-Man Chung wrote:
> Denis,
>
> I just had a chance to look at your patch, and I have couple of questions.
>
> In the generated method finallies, there is the code fragment,
>
> if (bitmask.get(1)) {
> if (((javax.servlet.jsp.tagext.BodyTag)tags.elementAt(1)).doAfterBody() !=
> javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUDE)
> out = pageContext.popBody();
> }
This code corresponds to the pseudo "finally" that can be found at lines 1068 to
1092. It is generated in the generateCustomEnd() method when the tag did have
a body and implemented the BodyTag interface.
>
> Shouldn't that be doStartTag() instead of doAfterBody()?
No because it correspond to the "finally" part of a try/finally block of a
Tag implementing the BodyTag interface.
>
> Also, if an exception occurs after the method is invoked, wouldn't it be
> invoked again in finallies? I think you need to pass the return value for
> the method to fanallies instead.
No, because there is no more try/finally except the "global" one. The idea
was to replace the begin of the "try" clause by setting a bit, and when the
begin of a "finally" clause is started to clear the bit. So if an exception
is raised after the "try" and before the "finally", the bit is set, in any
other condition the bit will be cleared. You'll notice that whatever would
be output in the "finally" clause is also added to the "finallies" method,
that's why if there is an ".doAfterBody()" in a "finally" clause you'll find
it also in the "finallies" method.
You'll notice that in the generated code, the patch leaves as comments the
begin of the "try", and the begin of the "finally" and their end. This way, it
is easy to validate the logic by following the flow in case an Exception is
raised somewhere. We can easily compare the code that would have been executed
with the "try/finally" versus the "finallies" method and the bits.
Mr Maucherat noticed that the patch do create a BitSet and a Vector, even for
JSPs that don't have tags, I think it could be avoided if we did some kind of
lazy initialisation. My first, dumb, I confess! idea was to put the Vector
and the BitSet as instance variables, but Mr Barker was quick to point out
that it would cause thread problem. My idea was then to have the patch mature
and if everything was ok to do a second pass to optimize it.
But we could certainly ask ourselves if this is the right approach to the
problem. We know that we can't afford deeply nested try/finally. The patch
was tought to disturb as little the control flow of generated page without the
performance impacts of the nested try/finally. Another approach would be to
think if the page could be generated entirely differently with just one
try/finally and still respect the specs.
Thanks!
--
Denis Benoit
benoitde@sympatico.ca
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>