Bug 57486

Summary: Improve reuse of ProtectedFunctionMapper instances in generated JSP
Product: Tomcat 8 Reporter: Konstantin Kolinko <knst.kolinko>
Component: JasperAssignee: Tomcat Developers Mailing List <dev>
Status: NEW ---    
Severity: enhancement CC: donnchadh
Priority: P2    
Version: 8.0.17   
Target Milestone: ----   
Hardware: PC   
OS: All   

Description Konstantin Kolinko 2015-01-23 08:50:00 UTC
The following was noted when reviewing r1653857 (the fix for bug 57441), r1653972.
I am filing it as an enhancement, as these are performance issues, not affecting observed functionality.

The code discussed here is org.apache.jasper.compiler.ELFunctionMapper$ELFunctionVisitor.doMap(ELNode.Nodes el).

Functionality
====
The code is responsible for generation of ProtectedFunctionMapper instances that are passed to EL expressions when evaluating them.

1) If EL expression does not call functions, it does not need a FunctionMapper, so none is generated.
2) If EL expression has function calls, it needs a FunctionMapper instance. The instance is populated with all functions that are called by this EL expression.

There is an attempt to reuse FunctionMapper instances  (ELFunctionVisitor.matchMap()), but in general each EL expression gets its own FunctionMapper instance. The class generates code that creates mapper instances and assigns them to static fields in JSP class.

Example
====
http://localhost:8080/examples/jsp/jsp2/el/functions.jsp

The "functions.jsp" example page generates the current code (with current Tomcat trunk) (several empty lines added for better readability):

[[[
private static org.apache.jasper.runtime.ProtectedFunctionMapper _jspx_fnmap_0;
private static org.apache.jasper.runtime.ProtectedFunctionMapper _jspx_fnmap_1;
private static org.apache.jasper.runtime.ProtectedFunctionMapper _jspx_fnmap_2;

static {
  _jspx_fnmap_0= org.apache.jasper.runtime.ProtectedFunctionMapper.getMapForFunction("fn:escapeXml", org.apache.taglibs.standard.functions.Functions.class, "escapeXml", new Class[] {java.lang.String.class});

  _jspx_fnmap_1= org.apache.jasper.runtime.ProtectedFunctionMapper.getInstance();
  _jspx_fnmap_1.mapFunction("my:reverse", jsp2.examples.el.Functions.class, "reverse", new Class[] {java.lang.String.class});
  _jspx_fnmap_1.mapFunction("fn:escapeXml", org.apache.taglibs.standard.functions.Functions.class, "escapeXml", new Class[] {java.lang.String.class});

  _jspx_fnmap_2= org.apache.jasper.runtime.ProtectedFunctionMapper.getInstance();
  _jspx_fnmap_2.mapFunction("my:countVowels", jsp2.examples.el.Functions.class, "numVowels", new Class[] {java.lang.String.class});
  _jspx_fnmap_2.mapFunction("fn:escapeXml", org.apache.taglibs.standard.functions.Functions.class, "escapeXml", new Class[] {java.lang.String.class});
}
]]]

The first one is mapper used for EL that calls a single function ("fn:escapeXml"). Other are for ELs that call two functions each.

Concerns
====
1. In JSP pages the tag libraries are declared only once. Thus it is possible to reuse a single ProtectedFunctionMapper instance across the entire JSP page.

Instead of the above 3 mapper instances it would be better to have one mapper instance that provides all ("fn:escapeXml", "my:reverse", "my:countVowels") at once.

In JSP documents (*.jspx) the libraries can be declared differently on each element, so situation will be more difficult.

I say that a function mapper can be reused if all functions declared in it use the same set of URIs (all used functions belong to the same set of tag libraries).  This is how ELFunctionVisitor.matchMap() has to be implemented.


When a function mapper is reused, it shall be possible to update is (add new function mappings). I imply that a ProtectedFunctionMapper shall be able to automatically switch from single-method to multiple-methods-map mode when its mapFunction() method is called to add a method.


2. Regarding r1653857
ELFunctionMapper$ELFunctionVisitor.doMap() uses nested Fvisitor class to get a list of functions called by EL.  The functions where getFunctionInfo() is null (Added via Lambda or ImportHandler) are later ignored by ProtectedFunctionMapper methods.

It would be more simple to skip them in the list generated by Fvisitor instead of skipping them later in ProtectedFunctionMapper.  There is no need to pass them to ProtectedFunctionMapper, as it will return null when being asked about them.