2

I was told to optimize a web application. To do so I use JProfiler.

I noticed a large part of the response time is spent in the presentation layer. Especially, when the code builds the HTML code.

Drilling down this bottle neck, I saw that the code deals a lot with OGNL classes/methods (nearly 50% of the calls).

I tuned the JSP code (see below for details) by removing some unnecessary OGNL statements. I managed to gain 200ms. However the code still spend more than 2000ms on average in this bottle neck. The overall response time is around 3000ms.

The OGNL documentation says this:

As of OGNL 2.5.0 the OgnlContext object can automatically tracks evaluations of expressions. (...) you can access the last evaluation through the lastEvaluation property of OgnlContext.

Since I use Struts 2.3.15.1 (OGNL v3.0.6) how can I access this property or call the getLastEvaluation of OgnlContext either from JSP file or Action class ?


JSP file

<%@ page language="java" contentType="text/html; charset=UTF-8" pageEncoding="UTF-8"%>
<%@ taglib prefix ="s" uri="/struts-tags" %>
<!DOCTYPE html>

<html>
<head>
<title><s:text name="domain.domain0.title"/></title>
<script type="text/javascript" src="<s:url value='/js/domain/domain0.js'/>"></script>
<script type="text/javascript" src="<s:url value='/js/core/html.js'/>"></script>
</head>
<body>
    <div style="width:100%;color:green;font-weight: bold;text-align:center">
        <s:text name="domain.domain0.information"/>
    </div>
    <div class="content selectFen">
        <s:set var="noAnchor">false</s:set>
        <s:iterator value="artefactList" status="numRow">
            <s:if test="court == nomenclatureCritere && #noAnchor!=true">
                <s:set var="noAnchor">true</s:set>
                <a id="NomencAnchor" href="#NomencAnchor" style="visibility:hidden;"></a>
            </s:if>
            <s:set var="isOpen" value="nomenclatureCritere.startsWith(court) || ligProd != '80'"/>
            <div class="parcoursNomenc">
                <table onmouseover='this.style.backgroundColor="gainsboro"' onmouseout='this.style.backgroundColor=""' style="width: 100%;font-weight:bolder">
                    <tbody>
                        <tr>
                            <s:url var="childNomenUrl" action="ajaxPopupNomenChild" escapeAmp="false">
                                <s:param name="dateCritere" value="dateCritere"/>
                                <s:param name="sidNomenclaturePere" value="sidArtNomenc"/>
                            </s:url>
                            <s:if test="#isOpen">
                               <s:if test="#numRow.last">
                                   <td onclick="noeudPlusMoins(this);loadChild('<s:property value="childNomenUrl" />', 'divParcours<s:property value="sidArtNomenc"/>')" class="open_last">&#160;</td>
                               </s:if>
                               <s:else>
                                   <td onclick="noeudPlusMoins(this);loadChild('<s:property value="childNomenUrl" />', 'divParcours<s:property value="sidArtNomenc"/>')" class="open">&#160;</td>
                               </s:else>
                            </s:if>
                            <s:else>
                               <s:if test="#numRow.last">
                                   <td onclick="noeudPlusMoins(this);loadChild('<s:property value="childNomenUrl" />', 'divParcours<s:property value="sidArtNomenc"/>')" class="closed_last">&#160;</td> 
                               </s:if>
                               <s:else>
                                   <td onclick="noeudPlusMoins(this);loadChild('<s:property value="childNomenUrl" />', 'divParcours<s:property value="sidArtNomenc"/>')" class="closed">&#160;</td>
                               </s:else>
                            </s:else>
                            <td style="vertical-align: top; width: 8.5em; padding-top: 5px;">
                                <s:text name="domain.domain0.label"/> <s:property value="court"/>
                            </td>
                            <td class="description">
                                <s:property value="description"/>
                            </td>
                        </tr>
                    </tbody>
                </table>
                <div id="divParcours<s:property value="sidArtNomenc"/>" class="<s:if test="numRow.last">subtree_last</s:if><s:else>subtree</s:else>" >
                   <s:if test="#isOpen">
                       <s:action name="popupNomenExist" executeResult="true"> 
                          <s:param name="dateCritere" value="dateCritere"/> 
                          <s:param name="nomenclatureCritere" value="nomenclatureCritere"/>
                       </s:action>
                       <s:action name="ajaxPopupNomenChild" executeResult="true"> 
                          <s:param name="dateCritere" value="dateCritere"/>
                          <s:param name="sidNomenclaturePere" value="sidArtNomenc"/>
                       </s:action>
                    </s:if>
                </div>
           </div>
        </s:iterator>
    </div>
</body>
</html>
Roman C
  • 49,761
  • 33
  • 66
  • 176
Stephan
  • 41,764
  • 65
  • 238
  • 329

2 Answers2

1

You can access OgnlContext like

OgnlContext context = (OgnlContext) ActionContext.getContext().getValueStack().getContext();
context.setTraceEvaluations(true); 
Evaluation evaluation = context.getLastEvaluation();

How to trace evaluations is described in the document you linked in the section for Trace Evaluations

You can also set system property ognl.traceEvaluations before the context is instantiated to turn on this feature.

Roman C
  • 49,761
  • 33
  • 66
  • 176
  • Is a ServletContextListener well suited for initializing the system property ? – Stephan Jun 06 '14 at 10:15
  • yes, you can do it on application startup, note you can also do it in `DispatcherListener`. One more thing about your JSP: get rid of `s:action` tags if you want better performance. – Roman C Jun 06 '14 at 10:25
  • What can I use instead of `s:action` tags ? Their performing job seem pretty hard if I must generate the complete url myself. – Stephan Jun 06 '14 at 10:36
  • Use action properties. – Roman C Jun 06 '14 at 10:38
  • I'm sorry, I didn't understand. Do you have any pointers or an example of action `properties`? – Stephan Jun 06 '14 at 11:11
  • Action property is a name exposed by the get/set method, it can also have a class member variable that holds a value, but it's not necessary. Action class is a JavaBean accessible via OGNL. OGNL also is Java enabled, you can use Java objects and its methods directly. Examples you can find many of them browsing my answers with ognl tag. – Roman C Jun 06 '14 at 11:31
  • 1
    @Stephan They are right, the s:action tags should be avoided at all costs. Each time that tag is used it is as if you called that action from the url (including interceptors), doing this inside your iterator on a row by row basis is _very_ expensive. – Quaternion Jun 06 '14 at 20:37
1

Looking at your JSP code, it's evident that the culprits are the <s:action> tags:

You call two actions on each iteration... that basing on the number of rows you are iterating, may become a serious problem.

I would try refactoring that section; a big part of the bottleneck is in the mechanism to interpret the tag, call the action, pass through the Interceptor Stack, and so on.

If you can generate the same output minimizing the calls to actions, the bottleneck would decrease significantly.

P.S: a very minimal optimization is to change the order of the boolean and string comparison in your s:if, to exploit the short circuit (boolean comparison is faster):
<s:if test="#noAnchor!=true && court == nomenclatureCritere">

EDIT

It works better without s:action however the page need them and I don't know how to replace them

To be more clear, an Action should

  • perform one (or few) logical action;
  • never be used as an helper class.

When you call popupNomenExist , your only requirements is to call the server, do something, and receive the result to inject it in the JSP.

There are several ways to receive that result, and calling an Action is probably the worst (in terms of performances, because of the reasons listed above).

You can take the code from the execute() method of your popupNomenExist Action, and :

  1. place it in a method of YOUR current action, like described in RomanC linked answer;
  2. place it in a static method of an helper class;
  3. precalculate it in your current action, and expose the results as List of Strings;
  4. and so on...

In addition, we can't argue what is exactly being performed inside your popupNomenExist's execute() method, but I guess there are chance that it could be refactored to prevent executing some part of logic that is common to each iteration, grouping them and executing before the iteration as a sort of cache.

Andrea Ligios
  • 49,480
  • 26
  • 114
  • 243
  • Is `!#noAnchor` faster than `#noAnchor!=true` ? – Stephan Jun 06 '14 at 10:43
  • No. I was meaning that since the two expression (boolean=boolean and object=object) are in AND, and thanks to the shortcircuit the second won't be evaluated if the first is false, placing the fastest expression as first will theoretically improve the performances. Practically, it won't :) But even if not noticeable on few iterations, it's just better, imho. – Andrea Ligios Jun 06 '14 at 12:15
  • Focus on the `` part, btw. Try removing both tags at first, and see if it drops from 2000ms to something like 200ms, to understand how much of the bottleneck is there. – Andrea Ligios Jun 06 '14 at 12:17
  • It works better without `s:action` however the page need them and I don't know how to replace them. The example from Roman C (http://stackoverflow.com/q/21170413/573032) didn't guide to a way for replace `s:action`s. – Stephan Jun 06 '14 at 13:12
  • @AndreaLigios Just one moment, I mentioned a link to the question, not an answer for the OP to better understand the problem. There's also useful comments under the question itself. BTW the OP's post is about OGNL context, which is a value stack context as you might see below, and how to get the trace evaluations. If this is a performance issue like some post nearby, It's unclear how many rows in the list and it also won't solve the performance issue itself. The code doesn't use a pagination, the iterator might iterate a billion records. There's no action code that prove it. – Roman C Jun 06 '14 at 16:51
  • I totally agree with you at half :) – Andrea Ligios Jun 06 '14 at 17:37
  • @Stephan did it worked ? Do you still have problems reimplementing that part of code ? – Andrea Ligios Jun 09 '14 at 15:47
  • @AndreaLigios I succeed in reducing the total consumed time, I'll post my findings later. However I still can't see how two get rid of the actions. It's still unclear for me how action properties should look. Do I have to call the `executeResult` method on the action class myself ? – Stephan Jun 18 '14 at 07:42