https://bz.apache.org/bugzilla/show_bug.cgi?id=69333

            Bug ID: 69333
           Summary: Unnecessary code in generated JSPs
           Product: Tomcat 9
           Version: 9.0.x
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Jasper
          Assignee: dev@tomcat.apache.org
          Reporter: jeng...@amazon.com
  Target Milestone: -----

Our large, high-traffic system has 1000+ jsps, and more than one class file is
> 1MB (not my fault :) ).  Examination of the generated code identified an
isolated changed that will slightly reduce code size, although with near-zero
runtime impact.  The primary benefit to our system will be smaller class files,
meaning lower aggregate code cache usage.

Referencing generated code such as this:

  private boolean
_jspx_meth_c_005fotherwise_005f30(javax.servlet.jsp.tagext.JspTag
_jspx_th_c_005fchoose_005f34, javax.servlet.jsp.PageContext _jspx_page_context,
int[] _jspx_push_body_count_c_005fforEach_005f21)
          throws java.lang.Throwable {
...
    _jsp_getInstanceManager().newInstance(_jspx_th_c_005fotherwise_005f30);
    try {
     ...
    } finally {
     
org.apache.jasper.runtime.JspRuntimeLibrary.releaseTag(_jspx_th_c_005fotherwise_005f30,
_jsp_getInstanceManager(), false);
    }
    return false;
  }

The finally block triggers the JSPRuntimeLibrary.releaseTag() method:

public static void releaseTag(Tag tag, InstanceManager instanceManager, boolean
reused) {
  // Caller ensures pool is non-null if reuse is true
  if (!reused) {
    releaseTag(tag, instanceManager);
  }
}

The generated JSP code includes the hard-coded value "false", which
short-circuits the releaseTag method and guarantees the line achieves nothing.

One can argue that the JIT compiler will notice this and remove it; however,
this line actually contains two method calls, JspRuntimeLibrary.releaseTag and
_jsp_getInstanceManager().


public org.apache.tomcat.InstanceManager _jsp_getInstanceManager() {
  if (_jsp_instancemanager == null) {
    synchronized (this) {
      if (_jsp_instancemanager == null) {
        _jsp_instancemanager =
org.apache.jasper.runtime.InstanceManagerFactory.getInstanceManager(getServletConfig());
      }
    }
  }
  return _jsp_instancemanager;
}

This method call will almost always return the cached value and therefore be a
very highly-optimized branch... however it must always be checked, and
therefore always be executed.  In addition, the JIT is not guaranteed to inline
the releaseTag() and _jsp_getInstanceManager() methods, in which case it cannot
detect the available optimizations.

Put it all together and this is an unnecessary line of code, repeated over and
over, that has a non-zero impact.  This analysis applies only when the
generated argument is "false", and this line is clearly required when set to
"true".

Preferred solution is to remove the line in all cases when the value is
"false".  If the try/finally can be simultaneously removed, that's even better,
and removes yet more instructions.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to