ASF Bugzilla – Bug 58327
ValueExpressionLiteral should cache the string value
Last modified: 2015-09-04 11:31:32 UTC
Created attachment 33061 [details] Proposed patch ValueExpressionLiteral invokes #getExpressionString() twice for many methods to notify the context. Calculating the string value can be expensive, for example in my case my values are rather "big" JSON wrapper objects. For a literal value this string value should be cacheable - the thing is read-only and a literal value, after all. Attached a proposed patch to implement that - but I'm not sure whether there might be specific tests to run or spec issues preventing this. For the time being I worked around the issue in our own code by using a different ValueExpression that has this caching behavior.
The only risk I can see is that even though the ValueExpression is read-only, the object it wraps is not and that object may change over time. That may change the result of its toString() method which in turn would impact the result of #getExpressionString(). However, the use of toString() here is an implementation detail internal to Tomcat. The spec has no expectations of what should be produced for #getExpressionString() in this case. On balance I think the potential performance benefits outweigh the possible confusion that might be caused by caching the toString() value if getExpressionString() is used for debugging. I'll look at getting this patch applied.
Thanks for the report and the patch. It has been applied to trunk and 8.0.x (for 8.0.27 onwards). I haven't applied it to 7.0.x since getExpressionString() is not used so extensively in 7.0.x.