2016-03-10 17:22 GMT+03:00 <ma...@apache.org>:
> Author: markt
> Date: Thu Mar 10 14:22:51 2016
> New Revision: 1734418
>
> URL: http://svn.apache.org/viewvc?rev=1734418&view=rev
> Log:
> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=57583
> Improve long standing performance issue with EL and undefined attributes.
>
> Modified:
> tomcat/trunk/java/javax/servlet/jsp/el/ScopedAttributeELResolver.java
> tomcat/trunk/java/org/apache/el/parser/AstIdentifier.java
> tomcat/trunk/webapps/docs/changelog.xml
>
> Modified:
> tomcat/trunk/java/javax/servlet/jsp/el/ScopedAttributeELResolver.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/javax/servlet/jsp/el/ScopedAttributeELResolver.java?rev=1734418&r1=1734417&r2=1734418&view=diff
> ==============================================================================
> --- tomcat/trunk/java/javax/servlet/jsp/el/ScopedAttributeELResolver.java
> (original)
> +++ tomcat/trunk/java/javax/servlet/jsp/el/ScopedAttributeELResolver.java Thu
> Mar 10 14:22:51 2016
> @@ -35,6 +35,20 @@ import javax.servlet.jsp.PageContext;
> */
> public class ScopedAttributeELResolver extends ELResolver {
>
> + // Indicates if a performance short-cut is available
> + private static final Class<?> AST_IDENTIFIER_KEY;
> +
> + static {
> + Class<?> key = null;
> + try {
> + key = Class.forName("org.apache.el.parser.AstIdentifier");
> + } catch (Exception e) {
> + // Ignore: Expected if not running on Tomcat. Not a problem since
> + // this just allows a short-cut.
> + }
> + AST_IDENTIFIER_KEY = key;
> + }
> +
> @Override
> public Object getValue(ELContext context, Object base, Object property) {
> if (context == null) {
> @@ -51,10 +65,26 @@ public class ScopedAttributeELResolver e
> result = page.findAttribute(key);
>
> if (result == null) {
> + boolean resolveClass = true;
> + // Performance short-cut available when running on Tomcat
> + if (AST_IDENTIFIER_KEY != null) {
> + // Tomcat will set this key to Boolean.TRUE if the
> + // identifier is a stand-alone identifier (i.e.
> + // identifier) rather than part of an AstValue (i.e.
> + // identifier.something). Imports do not need to be
> + // checked if this is a stand-alone identifier
> + Boolean value = (Boolean)
> context.getContext(AST_IDENTIFIER_KEY);
> + if (value != null && value.booleanValue()) {
> + resolveClass = false;
> + }
> + }
> // This might be the name of an imported class
> ImportHandler importHandler = context.getImportHandler();
> if (importHandler != null) {
> - Class<?> clazz = importHandler.resolveClass(key);
> + Class<?> clazz = null;
> + if (resolveClass) {
> + clazz = importHandler.resolveClass(key);
> + }
> if (clazz != null) {
> result = new ELClass(clazz);
> }
>
> Modified: tomcat/trunk/java/org/apache/el/parser/AstIdentifier.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/el/parser/AstIdentifier.java?rev=1734418&r1=1734417&r2=1734418&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/el/parser/AstIdentifier.java (original)
> +++ tomcat/trunk/java/org/apache/el/parser/AstIdentifier.java Thu Mar 10
> 14:22:51 2016
> @@ -77,7 +77,27 @@ public final class AstIdentifier extends
>
> // EL Resolvers
> ctx.setPropertyResolved(false);
> - Object result = ctx.getELResolver().getValue(ctx, null, this.image);
> + Object result;
> + /* Putting the Boolean into the ELContext is part of a performance
> + * optimisation for ScopedAttributeELResolver. When looking up "foo",
> + * the resolver can't differentiate between ${ foo } and ${ foo.bar
> }.
> + * This is important because the expensive class lookup only needs to
> + * be performed in the later case. This flag tells the resolver if
> the
> + * lookup can be skipped.
> + */
> + if (parent instanceof AstValue) {
> + ctx.putContext(this.getClass(), Boolean.FALSE);
> + } else {
> + ctx.putContext(this.getClass(), Boolean.TRUE);
> + }
Honestly, I do not understand the above if/else block.
I do not see how the difference of "${ foo } vs ${ foo.bar }" maps
into "(parent instanceof AstValue)" check.
What is the meaning of "parent" in Ast?
What possible values can it have?
Does this handle method calls, such as ${System.currentTimeMillis()} ?
> + try {
> + result = ctx.getELResolver().getValue(ctx, null, this.image);
> + } finally {
> + // Always reset the flag to false so the optimisation is not
> applied
> + // inappropriately
> + ctx.putContext(this.getClass(), Boolean.FALSE);
> + }
> +
> if (ctx.isPropertyResolved()) {
> return result;
> }
Below these lines the AstIdentifier does its own
ImportHandler.resolveClass(), resolveStatic() calls.
This duplicates the work performed by ScopedAttributeELResolver and I
wonder whether it is necessary.
Those lines originate from
http://svn.apache.org/viewvc?diff_format=l&view=revision&revision=1504286
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1734418&r1=1734417&r2=1734418&view=diff
> ==============================================================================
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Thu Mar 10 14:22:51 2016
> @@ -222,6 +222,12 @@
> <update>
> Update to the Eclipse JDT Compiler 4.5.1. (markt)
> </update>
> + <fix>
> + <bug>57583</bug>: Improve the performance of
> + <code>javax.servlet.jsp.el.ScopedAttributeELResolver</code> when
> + resolving attributes that do not exist. This improvement only works
> when
> + Jasper is used with with Tomcat's EL implementation. (markt)
> + </fix>
> </changelog>
> </subsection>
> <subsection name="WebSocket">
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
