Bug 54017 - new String instance is generated for constant string in Generator.convertString
new String instance is generated for constant string in Generator.convertString
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Jasper
trunk
PC All
: P2 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2012-10-17 08:47 UTC by Sheldon Shao
Modified: 2012-12-04 11:49 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sheldon Shao 2012-10-17 08:47:42 UTC
If the target class is "Object.class", the generator generates
        "new String(" + quoted + ")";
as attribute value for Tag Handler.

How about using quoted directly same as when the target class is "Object.class"?

Creating a String instance will cause some overhead from memory allocation and hash code recaluation when it is used as a key on HashMap.


Here is the detail code,

        /*
         * @param c The target class to which to coerce the given string @param
         * s The string value @param attrName The name of the attribute whose
         * value is being supplied @param propEditorClass The property editor
         * for the given attribute @param isNamedAttribute true if the given
         * attribute is a named attribute (that is, specified using the
         * jsp:attribute standard action), and false otherwise
         */
        private String convertString(Class<?> c, String s, String attrName,
                Class<?> propEditorClass, boolean isNamedAttribute) {

            String quoted = s;
            if (!isNamedAttribute) {
                quoted = quote(s);
            }

            if (propEditorClass != null) {
                String className = c.getCanonicalName();
                return "("
                        + className
                        + ")org.apache.jasper.runtime.JspRuntimeLibrary.getValueFromBeanInfoPropertyEditor("
                        + className + ".class, \"" + attrName + "\", " + quoted
                        + ", " + propEditorClass.getCanonicalName() + ".class)";
            } else if (c == String.class) {
                return quoted;
            } else if (c == boolean.class) {
                return JspUtil.coerceToPrimitiveBoolean(s, isNamedAttribute);
            } else if (c == Boolean.class) {
                return JspUtil.coerceToBoolean(s, isNamedAttribute);
            } else if (c == byte.class) {
                return JspUtil.coerceToPrimitiveByte(s, isNamedAttribute);
            } else if (c == Byte.class) {
                return JspUtil.coerceToByte(s, isNamedAttribute);
            } else if (c == char.class) {
                return JspUtil.coerceToChar(s, isNamedAttribute);
            } else if (c == Character.class) {
                return JspUtil.coerceToCharacter(s, isNamedAttribute);
            } else if (c == double.class) {
                return JspUtil.coerceToPrimitiveDouble(s, isNamedAttribute);
            } else if (c == Double.class) {
                return JspUtil.coerceToDouble(s, isNamedAttribute);
            } else if (c == float.class) {
                return JspUtil.coerceToPrimitiveFloat(s, isNamedAttribute);
            } else if (c == Float.class) {
                return JspUtil.coerceToFloat(s, isNamedAttribute);
            } else if (c == int.class) {
                return JspUtil.coerceToInt(s, isNamedAttribute);
            } else if (c == Integer.class) {
                return JspUtil.coerceToInteger(s, isNamedAttribute);
            } else if (c == short.class) {
                return JspUtil.coerceToPrimitiveShort(s, isNamedAttribute);
            } else if (c == Short.class) {
                return JspUtil.coerceToShort(s, isNamedAttribute);
            } else if (c == long.class) {
                return JspUtil.coerceToPrimitiveLong(s, isNamedAttribute);
            } else if (c == Long.class) {
                return JspUtil.coerceToLong(s, isNamedAttribute);
            } else if (c == Object.class) {
                return "new String(" + quoted + ")";
            } else {
                String className = c.getCanonicalName();
                return "("
                        + className
                        + ")org.apache.jasper.runtime.JspRuntimeLibrary.getValueFromPropertyEditorManager("
                        + className + ".class, \"" + attrName + "\", " + quoted
                        + ")";
            }
        }
Comment 1 Mark Thomas 2012-10-25 12:17:37 UTC
Yes, that makes sense since String is an instance of an Object there is no need to create a new String. Fixed for 7.0.x and trunk and will be included in 7.0.33 onwards.
Comment 2 Konstantin Kolinko 2012-12-04 11:49:48 UTC
Regarding this fix ( r1402122 r1402123 ),

The JSP 2.2 spec chapter JSP.1.14.2.1 "Conversions from String values"
defines conversion to an Object as

[quote]
"As if new String(string-literal). This results in new String("")
if the string is empty."
[/quote]

This should be the reason for the original code changed by this fix.

Actually I agree with OP and Mark and I see no good reason to use a String(String) constructor here.

The only good point I see in String(String) constructor is that it trims the backing char array to contain only the needed count of characters, but I do not think it is important here.