Ah, I forgot about the fix that you did in SmapUtil to improve line mappings
for template texts. Sorry. :-(
But this also shows that tight coupling between Generator and SmapUtil is
flagile and error prone. I think it would be a better design if we
decouple these two modules somehow. We could create additional data
structure that captures the mapping info for template texts, with Generator
its producer and SmapUtil its comsumer. What do you think? I'll
think about this more.
BTW, the reason for "if (textSize <= 3)" is for performance optimization.
"out.write('\n')" should be faster than "out.write("\n")" so it's OK that
you move into the "if (ctxt.getOptions().genStringAsCharArray())" block,
for now; but it should really be applied in all cases. Maybe we can
write all "out.write()"'s in a single line, so that the mapping is not
changed? Or we can revisit this later.
-Kin-man
> Date: Sun, 14 Sep 2003 21:44:56 -0700
> From: Eric Carmichael <[EMAIL PROTECTED]>
> Subject: Re: cvs commit:
jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/resources
messages.properties
> X-Originating-IP: [64.203.49.21]
> To: Tomcat Developers List <[EMAIL PROTECTED]>
> X-MIMEOLE: Produced By Microsoft MimeOLE V6.00.2800.1165
> X-Priority: 3
> X-MSMail-priority: Normal
> X-Originating-Email: [EMAIL PROTECTED]
> X-OriginalArrivalTime: 15 Sep 2003 04:45:06.0060 (UTC)
FILETIME=[22569CC0:01C37B44]
>
> This change leads to incorrect SMAPs of TemplateText nodes with 3 or fewer
> characters, because the "if (textSize <= 3)" logic isn't mirrored in
> SmapUtil.java.
>
> Moving the "if (textSize <= 3)" block into the "if
> (ctxt.getOptions().genStringAsCharArray())" block seems to be a quick way to
> fix the problem for genStrAsCharArray=false (which, if
> genStrAsCharArray=true is still experimental, is the only case that really
> matters), but I haven't followed this thread closely enough to feel
> confident committing that change.
>
> Eric
>
>
> ----- Original Message -----
> From: <[EMAIL PROTECTED]>
> To: <[EMAIL PROTECTED]>
> Sent: Tuesday, September 09, 2003 2:46 PM
> Subject: cvs commit:
> jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/resources
> messages.properties
>
>
> > kinman 2003/09/09 14:46:22
> >
> > Modified: jasper2/src/share/org/apache/jasper
> > EmbeddedServletOptions.java JspC.java Options.java
> > jasper2/src/share/org/apache/jasper/compiler Generator.java
> > jasper2/src/share/org/apache/jasper/resources
> > messages.properties
> > Log:
> > - Add an compilation option to generate writing char arrays instead of
> Strings
> > for template texts.
> >
> > OK, I'm committing the code experimentally. Please try it out and see
> > if it improves performance. To turn it on, set the initParam
> > genStrAsCharArray to true.
> >
> > Revision Changes Path
> > 1.6 +28 -3
> jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/EmbeddedServletOpt
> ions.java
> >
> > Index: EmbeddedServletOptions.java
> > ===================================================================
> > RCS file:
> /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/Embedded
> ServletOptions.java,v
> > retrieving revision 1.5
> > retrieving revision 1.6
> > diff -u -r1.5 -r1.6
> > --- EmbeddedServletOptions.java 6 Aug 2003 20:04:05 -0000 1.5
> > +++ EmbeddedServletOptions.java 9 Sep 2003 21:46:22 -0000 1.6
> > @@ -157,6 +157,11 @@
> > private boolean isSmapDumped = false;
> >
> > /**
> > + * Are Text strings to be generated as char arrays?
> > + */
> > + private boolean genStringAsCharArray = false;
> > +
> > + /**
> > * I want to see my generated servlets. Which directory are they
> > * in?
> > */
> > @@ -290,6 +295,13 @@
> > }
> >
> > /**
> > + * Are Text strings to be generated as char arrays?
> > + */
> > + public boolean genStringAsCharArray() {
> > + return this.genStringAsCharArray;
> > + }
> > +
> > + /**
> > * Class ID for use in the plugin tag when the browser is IE.
> > */
> > public String getIeClassId() {
> > @@ -513,6 +525,19 @@
> > } else {
> > if (log.isWarnEnabled()) {
> >
> log.warn(Localizer.getMessage("jsp.warning.dumpSmap"));
> > + }
> > + }
> > + }
> > +
> > + String genCharArray =
> config.getInitParameter("genStrAsCharArray");
> > + if (genCharArray != null) {
> > + if (genCharArray.equalsIgnoreCase("true")) {
> > + genStringAsCharArray = true;
> > + } else if (genCharArray.equalsIgnoreCase("false")) {
> > + genStringAsCharArray = false;
> > + } else {
> > + if (log.isWarnEnabled()) {
> > +
> log.warn(Localizer.getMessage("jsp.warning.genchararray"));
> > }
> > }
> > }
> >
> >
> >
> > 1.60 +10 -3
> jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/JspC.java
> >
> > Index: JspC.java
> > ===================================================================
> > RCS file:
> /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/JspC.jav
> a,v
> > retrieving revision 1.59
> > retrieving revision 1.60
> > diff -u -r1.59 -r1.60
> > --- JspC.java 2 Sep 2003 21:40:00 -0000 1.59
> > +++ JspC.java 9 Sep 2003 21:46:22 -0000 1.60
> > @@ -408,6 +408,13 @@
> > return false;
> > }
> >
> > + /**
> > + * Are Text strings to be generated as char arrays?
> > + */
> > + public boolean genStringAsCharArray() {
> > + return false;
> > + }
> > +
> > public String getIeClassId() {
> > return ieClassId;
> > }
> >
> >
> >
> > 1.18 +8 -3
> jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/Options.java
> >
> > Index: Options.java
> > ===================================================================
> > RCS file:
> /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/Options.
> java,v
> > retrieving revision 1.17
> > retrieving revision 1.18
> > diff -u -r1.17 -r1.18
> > --- Options.java 2 Sep 2003 21:40:00 -0000 1.17
> > +++ Options.java 9 Sep 2003 21:46:22 -0000 1.18
> > @@ -191,4 +191,9 @@
> > * Obtain a Tag Plugin Manager
> > */
> > public TagPluginManager getTagPluginManager();
> > +
> > + /**
> > + * Are Text strings to be generated as char arrays?
> > + */
> > + public boolean genStringAsCharArray();
> > }
> >
> >
> >
> > 1.205 +91 -11
> jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler/Generator
> .java
> >
> > Index: Generator.java
> > ===================================================================
> > RCS file:
> /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/compiler
> /Generator.java,v
> > retrieving revision 1.204
> > retrieving revision 1.205
> > diff -u -r1.204 -r1.205
> > --- Generator.java 4 Sep 2003 22:30:06 -0000 1.204
> > +++ Generator.java 9 Sep 2003 21:46:22 -0000 1.205
> > @@ -70,6 +70,7 @@
> > import java.util.Collections;
> > import java.util.Enumeration;
> > import java.util.Hashtable;
> > +import java.util.HashMap;
> > import java.util.Iterator;
> > import java.util.List;
> > import java.util.Vector;
> > @@ -114,6 +115,7 @@
> > private PageInfo pageInfo;
> > private int maxTagNesting;
> > private Vector tagHandlerPoolNames;
> > + private GenBuffer charArrayBuffer;
> >
> > /**
> > * @param s the input string
> > @@ -154,6 +156,27 @@
> > }
> >
> > /**
> > + * Single quote and escape a character
> > + */
> > + static String quote(char c) {
> > +
> > + StringBuffer b = new StringBuffer();
> > + b.append('\'');
> > + if (c == '\'')
> > + b.append('\\').append('\'');
> > + else if (c == '\\')
> > + b.append('\\').append('\\');
> > + else if (c == '\n')
> > + b.append('\\').append('n');
> > + else if (c == '\r')
> > + b.append('\\').append('r');
> > + else
> > + b.append(c);
> > + b.append('\'');
> > + return b.toString();
> > + }
> > +
> > + /**
> > * Generates declarations. This includes "info" of the page
> directive,
> > * and scriptlet declarations.
> > */
> > @@ -714,6 +737,8 @@
> > private int methodNesting;
> > private TagInfo tagInfo;
> > private ClassLoader loader;
> > + private int charArrayCount;
> > + private HashMap textMap;
> >
> > /**
> > * Constructor.
> > @@ -725,6 +750,7 @@
> > FragmentHelperClass fragmentHelperClass,
> > ClassLoader loader,
> > TagInfo tagInfo) {
> > +
> > this.isTagFile = isTagFile;
> > this.out = out;
> > this.methodsBuffered = methodsBuffered;
> > @@ -734,20 +760,21 @@
> > methodNesting = 0;
> > handlerInfos = new Hashtable();
> > tagVarNumbers = new Hashtable();
> > + textMap = new HashMap();
> > }
> >
> > /**
> > * Returns an attribute value, optionally URL encoded. If
> > - * the value is a runtime expression, the result is the
> expression
> > - * itself, as a string. If the result is an EL expression,
> we insert
> > - * a call to the interpreter. If the result is a Named
> Attribute
> > - * we insert the generated variable name. Otherwise the
> result is a
> > - * string literal, quoted and escaped.
> > - *
> > + * the value is a runtime expression, the result is the
> expression
> > + * itself, as a string. If the result is an EL expression, we
> insert
> > + * a call to the interpreter. If the result is a Named
> Attribute
> > + * we insert the generated variable name. Otherwise the result
> is a
> > + * string literal, quoted and escaped.
> > + *
> > * @param attr An JspAttribute object
> > * @param encode true if to be URL encoded
> > - * @param expectedType the expected type for an EL
> evaluation
> > - * (ignored for attributes that aren't EL
> expressions)
> > + * @param expectedType the expected type for an EL evaluation
> > + * (ignored for attributes that aren't EL expressions)
> > */
> > private String attributeValue(
> > Node.JspAttribute attr,
> > @@ -1836,6 +1863,53 @@
> >
> > String text = n.getText();
> >
> > + int textSize = text.length();
> > + if (textSize == 0) {
> > + return;
> > + }
> > +
> > + if (textSize <= 3) {
> > + // Spcial case small text strings
> > + n.setBeginJavaLine(out.getJavaLine());
> > + out.printil("out.write(" + quote(text.charAt(0)) +
> ");");
> > + if (textSize > 1) {
> > + out.printil("out.write(" + quote(text.charAt(1)) +
> ");");
> > + }
> > + if (textSize > 2) {
> > + out.printil("out.write(" + quote(text.charAt(2)) +
> ");");
> > + }
> > + n.setEndJavaLine(out.getJavaLine());
> > + return;
> > + }
> > +
> > + if (ctxt.getOptions().genStringAsCharArray()) {
> > + // Generate Strings as char arrays, for performance
> > + ServletWriter caOut;
> > + if (charArrayBuffer == null) {
> > + charArrayBuffer = new GenBuffer(null);
> > + caOut = charArrayBuffer.getOut();
> > + caOut.pushIndent();
> > + textMap = new HashMap();
> > + } else {
> > + caOut = charArrayBuffer.getOut();
> > + }
> > + String charArrayName = (String) textMap.get(text);
> > + if (charArrayName == null) {
> > + charArrayName = "_jspx_char_array_" +
> charArrayCount++;
> > + textMap.put(text, charArrayName);
> > + caOut.printin("static char[] ");
> > + caOut.print(charArrayName);
> > + caOut.print(" = ");
> > + caOut.print(quote(text));
> > + caOut.println(".toCharArray();");
> > + }
> > +
> > + n.setBeginJavaLine(out.getJavaLine());
> > + out.printil("out.write(" + charArrayName + ");");
> > + n.setEndJavaLine(out.getJavaLine());
> > + return;
> > + }
> > +
> > n.setBeginJavaLine(out.getJavaLine());
> >
> > out.printin();
> > @@ -3029,6 +3103,11 @@
> > out.printMultiLn(fragmentHelperClass.toString());
> > }
> >
> > + // Append char array declarations
> > + if (charArrayBuffer != null) {
> > + out.printMultiLn(charArrayBuffer.toString());
> > + }
> > +
> > // Close the class definition
> > out.popIndent();
> > out.printil("}");
> > @@ -3078,6 +3157,7 @@
> > Generator(ServletWriter out, Compiler compiler) {
> > this.out = out;
> > methodsBuffered = new ArrayList();
> > + charArrayBuffer = null;
> > err = compiler.getErrorDispatcher();
> > ctxt = compiler.getCompilationContext();
> > fragmentHelperClass =
> >
> >
> >
> > 1.133 +3 -1
> jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/resources/messages
> .properties
> >
> > Index: messages.properties
> > ===================================================================
> > RCS file:
> /home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/resource
> s/messages.properties,v
> > retrieving revision 1.132
> > retrieving revision 1.133
> > diff -u -r1.132 -r1.133
> > --- messages.properties 25 Aug 2003 22:25:55 -0000 1.132
> > +++ messages.properties 9 Sep 2003 21:46:22 -0000 1.133
> > @@ -150,6 +150,8 @@
> > jsp.warning.development=Warning: Invalid value for the initParam
> development. Will use the default value of \"true\"
> > jsp.warning.fork=Warning: Invalid value for the initParam fork. Will
> use the default value of \"true\"
> > jsp.warning.reloading=Warning: Invalid value for the initParam
> reloading. Will use the default value of \"true\"
> > +jsp.warning.dumpSmap=Warning: Invalid value for the initParam dumpSmap.
> Will use the default value of \"false\"
> > +jsp.warning.genchararray=Warning: Invalid value for the initParam
> genStrAsCharArray. Will use the default value of \"false\"
> > jsp.warning.suppressSmap=Warning: Invalid value for the initParam
> suppressSmap. Will use the default value of \"false\"
> > jsp.error.badtaglib=Unable to open taglibrary {0} : {1}
> > jsp.error.badGetReader=Cannot create a reader when the stream is not
> buffered
> >
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > For additional commands, e-mail: [EMAIL PROTECTED]
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
