[
https://issues.apache.org/jira/browse/WW-3750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13194624#comment-13194624
]
Pelladi Gabor commented on WW-3750:
-----------------------------------
Indeed, an instance of ScopesHashModel is not thread-safe. But as I looked
through the source I have concluded that it doesn't needs to be thread-safe.
Here is what I did. I opened the call hierarchy for the constructor of
ScopesHashModel. This gets called only from
FreemarkerManager.buildScopesHashModel(), and that from
FreemarkerManager.buildTemplateModel(). From here there are two paths:
1. FreemarkerResult.createModel(), then FreemarkerResult.doExecute(). Here the
ScopesHashModel is assigned to a local variable, and after exiting the method,
the ScopesHashModel will be garbage collected.
2. FreemarkerTemplateEngine.renderTemplate(). Here the situation is similar,
the ScopesHashModel is a local variable and will be garbage collected.
So it seems that an instance of ScopesHashModel is not used by multiple
threads. For every struts2 tag rendering, a new instance is created. An other
field of this class, unlistedModels is also a simple HashMap, so the class is
not totally thread-safe even in it's current form.
But it is true that there may be a plugin or any code using this class in a way
that multiple threads operate on the same instance. The base class,
freemarker.template.SimpleHash is a bit more thread safe, javadoc says: "This
class is thread-safe if you don't call the put or remove methods after you have
made the object available for multiple threads.".
So I have created another patch that uses a ConcurrentHashMap. For this the
code had to be modified also, because ConcurrentHashMap (unlike HashMap) does
not handle null values.
> ScopesHashModel calls OgnlValueStack.findValue too many times during
> rendering freemarker templates
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3750
> URL: https://issues.apache.org/jira/browse/WW-3750
> Project: Struts 2
> Issue Type: Improvement
> Components: Value Stack
> Affects Versions: 2.3.1.1
> Reporter: Pelladi Gabor
> Priority: Minor
> Labels: patch, performance
> Fix For: 2.3.2
>
> Attachments: WW-3750.diff
>
>
> I saw using a profiler, that OgnlValueStack.findValue(String) gets called
> about a thousand times during rendering a page. Most of the calls are coming
> from ScopesHashModel.
> All freemarker templates contain a lot of references to e.g. "parameters".
> This variable is evaluated in ScopesHashModel over and over again, which
> takes about 10% time of total page load.
> I think we can assume, that the top-level objects on the value stack will not
> change during rendering a single struts2 tag. So with a little caching, we
> can eliminate most of the findValue method calls.
> In my application I tested this modification and didn't notice any side
> effects or bugs. The OgnlValueStack.findValue(String) calls on a test page
> went down from a thousand to a hundred. This improved overall page rendering
> time from about 400ms to 360ms.
> Patch is attached.
> Please review it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
