|Summary:||Minor performance enhancement in set result evaluation|
|Product:||Taglibs||Reporter:||Jeremy Boynes <jboynes>|
|Component:||Standard Taglib||Assignee:||Tomcat Developers Mailing List <dev>|
|Attachments:||patch to remove one check and a unneeded call to BodyContent#getString|
Description Jeremy Boynes 2010-07-03 13:26:05 UTC
Eliminate one comparison when evaluating body content as the content of "value" attribute can always be used when it has been specified. Jasper's implementation of BodyContent#getString() constructs a new value each time so only call it once to avoid an unnecessary object creation.
Comment 1 Jeremy Boynes 2010-07-03 13:27:20 UTC
Created attachment 25692 [details] patch to remove one check and a unneeded call to BodyContent#getString
Comment 2 Henri Yandell 2010-07-03 14:13:05 UTC
There's a functionality change here: If valueSpecified is false, yet there is a value, it used to return value. Now it returns bodyContent based results. I don't see this being in the spec, so presumably this is bad design that we're stuck with supporting; a protected status variable in addition to the main one, rather than a 'isValueSpecified()' method. Thoughts?
Comment 3 Jeremy Boynes 2010-07-03 14:53:00 UTC
value and valueSpecified are both protected variables and part of our implementation rather than the spec. The container will just call setValue(Object) to set the attribute's value and both our subclasses set both variables in their implementation. My thought was that the contract this has with its subclasses is that both must be set and if that is met the change works. The original tests I added weren't doing that which was why it was OK to change them.
Comment 4 Henri Yandell 2010-07-03 15:35:24 UTC
Agreed that the contract would be that both must be set. I've added javadoc that the two attributes must be in sync as a part of this commit: svn ci -m "Improving the performance of SetSupport by only calling bodyContent.getString() once per Jeremy's patch #49547"Sending impl/src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java Sending impl/src/test/java/org/apache/taglibs/standard/tag/common/core/TestSetSupport.java Transmitting file data .. Committed revision 960259.