On 10/03/2016 15:02, Konstantin Kolinko wrote: > 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.
<snip/> >> 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? It is the parent node of this node in the parse tree. > What possible values can it have? In theory, an instance of anything that extends SimpleNode. In practice, we are only interested in instances of AstValue since that is the type we will see when we need to check if the identifier represents a class and do the expensive class lookup. > Does this handle method calls, such as ${System.currentTimeMillis()} ? Yes. >> + 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. Yes it is necessary. ScopedAttributeELResolver is defined to always resolve the property (so it has to do these lookups itself) but most resolvers don't. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
