Created attachment 37726 [details] Code references - examples and partial solution Multiple of our large, publicly-facing applications contains thousands of JSPs. Some are executed more than others but all of the classes are loaded into memory for the lifetime of the application. This has stressed the JVM in various ways and we identified several ways to improve the generated classes (listed below). Note that I am describing the output of the JSP compilation, and not the compilation itself. The impact of our in-house fix (Tomcat8) depended on the application but in general we saw: Output .class files decreased in size by 20 - 40% JVM metaspace decreased by 20 - 90MB Code cache size decreased by 5-10% Small latency decrease (single-digit milliseconds) Small cpu decrease (present but difficult to quantify) Here are the issues we'd like to see addressesd: 1. Near-duplicate methods: each call to <c:set> generated its own method, even when a single property varied. For example <c:set var="test" value="${test1}"/> <c:set var='test" value="${test2}"/> generates two distinct methods (such as "_jspx_meth_c_005fset_005f2") even though they vary only by a constant string. Generating a single method (such as "_callTag_org_apache_taglibs_standard_tag_rt_core_SetTag_With_var_And_value") and parameterizing the value property eliminates one method for each matching instance within the JSP. 2. Tag pooling is disabled in all of our applications, and that value does not change at runtime - however tag pooling logic is executed anyway. This is generally trivial logic but it is executed many, many times... and it is generated many, many times. This is wasteful at execution time and also makes the methods larger than necessary. 3. There is wasteful method overhead such as aliasing parameters (_jspx_page_context -> pageContext) and an unused call to _jspx_page_context.getOut(). The JIT compiler will clean up much of that at runtime, however this is still present in the class file and therefore adds load to various native spaces, creates extra work during warmup (before JIT kicks in), and makes the JIT operations a bit slower. These optimizations are not significantly impactful to a single JSP, however they do add up in large applications, as described above. The attached file contains several methods: 1. _jspx_meth_c_005fset_005f2() - a generated method from a typical <c:set> 2. _jspx_meth_c_005fset_005f3() - a generated method from a <c:set> with a body. This illustrates the significant differences when a tag contains a body; our fix avoided this by leaving body tags as-is and only modifying empty tags. 3. _callTag_org_apache_taglibs_standard_tag_rt_core_SetTag_With_var_And_value() - a generated method from our fixed code that generically handles all <c:set value="" var=""/> tags. This method is reproduced in all of our JSPs with one or more occurrences of that tag (with no body!) and is referenced as many as 150 times. Note that the line count is reduced from 21 to 8. 4. generateOptimizedTagCall() - this is from our internal fix and is responsible for generating the reusable methods, such as the immediately previous example.
Moving to enhancement to set expectations correctly about likely implementation timescales. (We don't aim to fix every open enhancement for the next release round.)
Can you clarify what you mean in point 2? It isn't clear to me which logic you are referring to. An example would probably be best.
Sure! Tag pooling generates several lines of code in the compiled JSP; the lines below are copied from the attached source files, at the tail end of the "Old-style generated code" blocks. The code is nearly identical in all such methods, varying only by the auto-generated variable names. _005fjspx_005ftagPool_005fc_005fset_0026_005fvar_005fvalue_005fnobody.reuse(_jspx_th_c_005fset_005f2); _jspx_th_c_005fset_005f2_reused = true; } finally { org.apache.jasper.runtime.JspRuntimeLibrary.releaseTag(_jspx_th_c_005fset_005f2, _jsp_getInstanceManager(), _jspx_th_c_005fset_005f2_reused); }
Scratch that. The first example I looked at didn't have any pooling code but another one did. I see the issue. Regarding point 3, those parameters are required in some circumstances. I'm currently looking to see if the JSP code generation has enough information to be more selective about when they are declared. Generally, using Tomcat's proprietary tag plugin system is the best way to optimise tag code generation. There isn't much documentation available. All it does is point to the JSTL example proided with Tomcat: https://github.com/apache/tomcat/tree/master/java/org/apache/jasper/tagplugins/jstl
I can't repeat issue 2. If I clean out the work directory, disable tag pooling in conf/web.xml and then start Tomcat, the generated code doesn't have the *_reuse variable.
Hmm. 1. Tomcat 8? 2. We usually precompile our JSPs, perhaps there is something dynamic that handles it and we never noticed?
The setting at pre-compilation would control whether pooling was enabled or not. You'd need to call JspC with : ... -poolingEnabled false ... to disable tag pooling. Note that is all case sensitive. Still looking into what is possible to address issue 3.
Thank you, I've confirmed we are missing the JspC parameter, and we are correcting that in our apps.
Just a heads up that I am going to be spending some time improving the code coverage of the unit tests for the Generator class before looking at options to address issues 1 and 3. There are a lot of edge cases being handled in that class and I want to cover more of them with tests before I start changing things.
Good plan... our initial deployment had to be rolled back because I broke some weird syntax in tag fragments.
It has taken a while but I have just committed additional tests that bring the code coverage for Generator close to 100%. Next steps will be looking at the optimisations. The work on the tests identified some unnecessary code in Generator which has been removed but I don't expect that to have had a measurable performance impact.
Regarding issue 3: The call to _jspx_page_context.getOut() is now only made when required. The variable aliasing is more difficult. For more background see this thread https://markmail.org/thread/7hksrkahpvjg3ukz but the short version is: it is intentional; removing it isn't an option; and making it configurable is relatively risky/invasive for the associated benefit. Therefore, we do not currently plan to make any changes in this area.
I've spent some time this afternoon looking at issue 1. Tomcat's tagPlugin mechanism is the supported way to approach this. It lets you generate your own optimised code for tags. It is pretty flexible so you could have one plugin per tag or one plugin for a group of tags. There are some example implementations in org.apache.jasper.tagplugins.jstl to give you an idea of what is possible. The JSP samples in the examples webapp also include some tag plugins although it is worth noting that these are not enabled to use the plug-ins as the WEB-INF/tagPlugins.xml file is not present. As an example of how you could reduce simple <c:set .../> calls to a single line of Java see the sample plugin attached.
Created attachment 37867 [details] Sample plug-in for <c:set .../>
Sorry, I'm a little confused - are you planning to generate plugins for every tag? I view this more as an optimization to the global JSP compiler.
My expectation was that you'd use the plug-in mechanism to implement the optimisation for <c:set ... /> described in issue 1. If you want to provide a generic optimisation to multiple/all tags then that probably won't scale in your use case as you'd have to predefine the plugin mapping for each tag class. I'm also not sure the current API exposes enough information for a generic, rather than tag specific, solution. I'll take another look at issue 1 but I'd like to check my understanding of the generic problem first. Is this correct: Where: - multiple, non-pooled instances of the same tag are used in a JSP - the tags do not have a body - the tags have the same, single attribute set although the attribute may have different values for each tag instance then generate a single, parameterised method to be used for all tag instances rather than 1 method per tag instance?
Our Tomcat8 implementation modified Generator.java along the following lines: 1. When a tag call is found (any tag!): - generate a method name that uniquely identifies the tag class and the provided attributes - add the name/details to a list - generate code that calls the named method 2. Near the end of the file (after all calls are processed): - iterate through the loop - generate each method The sample code attachment includes the method generateOptimizedTagCall(). Let me know if you'd like to see more snippets... sorry, I know it's hard to read only pieces of the whole. The end result is that all JSPs everywhere become smaller, with zero application change or even activation required. I only see disadvantages with a tag-specific, application-specific solution.
These are good ideas, especially the Objective-C-style _callTag_org_apache_taglibs_standard_tag_rt_core_SetTag_With_var_And_value method generation and invocation. The biggest problem with Java-code generation from JSPs these days is the amount of code being generated into a single Java method. It's not uncommon to have so must stuff in a JSP that at least one of the generated-methods hits the 64kb limit. So anything that can reduce the amount of generated code in the _jsp_service method should be a priority.
We had a working version that chunked the jspService method, but dropped it because our use cases didn't need it... and the whole package was already pretty complex. Unfortunately I can't find the source. What I remember: 1. Generate the overhead (aliasing, try/catch, etc.) 2. Call a chunk1() method, with content generated later 3. Store up tags, output, etc. for the not-yet-created chunk1 4. Every so often (5 tags, 30 branches, whatever) switch to chunk2, 3, etc. 5. At the end, iterate through the accumulated chunk info (step #3 above) and generate the methods. Memory usage wasn't bad in our cases, but if necessary the chunkN() methods could be written to temp files then copied in when ready. One comment: "So anything that can reduce the amount of generated code in the _jsp_service method should be a priority." There seems to be tension between improvement for all cases, vs. support for an extreme case. A stronger argument for this concern is to maintain backwards compatibility (which I agree with 100%).